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

Add support to spec for config files from sources #247

Merged
merged 2 commits into from
May 14, 2024

Conversation

jeremyrickard
Copy link
Contributor

What this PR does / why we need it:

This PR adds the ability to define config files for the resulting artifact and the relevant %config entries in the %files section, It also supports the optional inclusion of the %config(noreplace) with a boolean in the config.

This also updates the spec to add this (and directories) to the IsEmpty method for the Artifact.

Which issue(s) this PR fixes (optional, using fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when the PR gets merged):
Fixes #246

Special notes for your reviewer:

Signed-off-by: Jeremy Rickard <jrickard@microsoft.com>
@jeremyrickard jeremyrickard requested a review from a team as a code owner May 13, 2024 19:16
spec.go Outdated
@@ -129,7 +129,9 @@ type Artifacts struct {
Manpages map[string]ArtifactConfig `yaml:"manpages,omitempty" json:"manpages,omitempty"`
// Directories is a list of various directories that should be created by the RPM.
Directories *CreateArtifactDirectories `yaml:"createDirectories,omitempty" json:"createDirectories,omitempty"`
// TODO: other types of artifacts (systtemd units, libexec, configs, etc)
// ConfigFiles is a list of files that should be marked as config files in the RPM.
Copy link
Member

Choose a reason for hiding this comment

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

Missed the in the RPM part in the last PR.
Should be in the package

Copy link
Contributor Author

Choose a reason for hiding this comment

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

doh updated this and the directories string.

spec.go Outdated
// This is used to customize the settings for a config entry in the resulting artifact.
type ArtifactConfigFileConfig struct {
ArtifactConfig
ReplaceOnUpdate bool `yaml:"replace,omitempty" json:"replace,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

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

Can you add a doc string for this?

Copy link
Member

Choose a reason for hiding this comment

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

Actually reading up on %config(noreplace), I think we should just assume this and use a normal ArtifactConfig.
If we need to tweak in the future we can add an option but I'm thinking %config(noreplace) is the normal correct behavior.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sounds good to me, made this change.

spec.go Outdated
// ArtifactConfigFileConfig is the configuration give for Artifact Config Files.
// This is used to customize the settings for a config entry in the resulting artifact.
type ArtifactConfigFileConfig struct {
ArtifactConfig
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
ArtifactConfig
ArtifactConfig `yaml:",inline"`

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've removed this entire struct now in favor of just Artifactconfig.

Signed-off-by: Jeremy Rickard <jrickard@microsoft.com>
@cpuguy83 cpuguy83 merged commit 1df475d into Azure:main May 14, 2024
9 checks passed
@cpuguy83 cpuguy83 added this to the v0.4.0 milestone May 14, 2024
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.

[REQ] Provide ability to define config files from sources
2 participants