Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

osbuild: unify implementations of files input 🧹 #3248

Merged
merged 6 commits into from
Jan 30, 2023

Conversation

thozza
Copy link
Member

@thozza thozza commented Jan 27, 2023

This PR is a pre-work to support custom files in /etc, because the copy stage implementation will need to be extended to support files input with origin source. Instead of implementing 6th version of it, I decided to rework files input implementation, which we wanted to do anyway (me and @achilleas-k).

Rework files input implementation to support all reference types supported by the input schema. Also implement helper functions to generate supported reference types. In some cases, the reference supports e.g. referencing multiple pipelines in the stage inputs, but this is currently not implemented, since no pipeline in composer uses it.

Rework the stage inputs implementation of the following stages to use the common files input:

  • XZ stage
  • FDO stage
  • QEMU stage
  • RPM stage
  • Ignition stage

This rework should not have any effect on manifests produced by osbuild-composer.

This pull request includes:

  • adequate testing for the new functionality or fixed issue
  • adequate documentation informing people about the change such as

@thozza thozza marked this pull request as draft January 27, 2023 10:43
@thozza thozza force-pushed the files-inputs-rework branch 2 times, most recently from d6bd1ae to 63f1625 Compare January 27, 2023 10:49
@thozza thozza marked this pull request as ready for review January 27, 2023 10:54
@7flying
Copy link
Member

7flying commented Jan 27, 2023

(just butting in to say that this makes the files input handling more easy to understand, and it would had made my life easier handling the Ignition stuff, so thanks!!)

achilleas-k
achilleas-k previously approved these changes Jan 27, 2023
Copy link
Member

@achilleas-k achilleas-k left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great stuff. Love how the stages cleaned up now. All is how it should be, for files inputs at least.

Rework files input implementation to support all reference types
supported by the input schema. Also implement helper functions to
generate supported reference types. In some cases, the reference
supports e.g. referencing multiple pipelines in the stage inputs, but
this is currently not implemented, since no pipeline in composer uses
it.

Rework the files input unit tests to cover the new functionality.

Adjust all code affected by the changes made to files input.

Signed-off-by: Tomáš Hozza <thozza@redhat.com>
The `FilesInputs` was since the beginning an XZ-specific implementation
of the input, but it was implemented in the `files_input.go` in a false
hope that it could be used as a generic stage inputs by any stages. It
turned out that various stages require different implementation of
its input. Specifically there is usually a stage-specific key, which has
assigned a common input type. For XZ stage, the key is `file`.

Remove `FilesInputs` and instead implement `XzStageInputs` which is now
accepted by the XZ stage.

Fix all affected pipeline implementations that use XZ stage.

Signed-off-by: Tomáš Hozza <thozza@redhat.com>
Rework the stage to not reimplement `FilesInput` as
`FDOStageInput`, but instead use the one common
`FilesInput` implementation and its supported
references.

Signed-off-by: Tomáš Hozza <thozza@redhat.com>
Rework the stage to not reimplement `FilesInput` as `QEMUStageInput`,
but instead use the one common `FilesInput` implementation and its
supported references.

Signed-off-by: Tomáš Hozza <thozza@redhat.com>
Rework the stage to not reimplement `FilesInput` as
`IgnitionStageInput`, but instead use the one common
`FilesInput` implementation and its supported
references.

Signed-off-by: Tomáš Hozza <thozza@redhat.com>
Rework the stage to not reimplement `FilesInput` as
`RPMStageInput`, but instead use the one common
`FilesInput` implementation and its supported
references.

Signed-off-by: Tomáš Hozza <thozza@redhat.com>
@thozza
Copy link
Member Author

thozza commented Jan 27, 2023

I added comments about the sha256sum and rebased the PR on main.

Copy link
Member

@achilleas-k achilleas-k left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still great :)

@achilleas-k achilleas-k merged commit 1667e6e into osbuild:main Jan 30, 2023
@thozza thozza deleted the files-inputs-rework branch January 30, 2023 10:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants