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

config/types: prevent files/links/dirs from conflicting with systemd units or dropins #1456

Merged
merged 1 commit into from
Nov 8, 2022

Conversation

marmijo
Copy link
Contributor

@marmijo marmijo commented Sep 1, 2022

Fixes Ignition#881.

Users can specify files/links/dirs within /etc/systemd/system that would conflict with systemd units or dropin files at the same location. Add validation to prevent this configuration conflict.

See machine-config-operator#1203 for the decision to add this validation to Ignition.

@marmijo
Copy link
Contributor Author

marmijo commented Sep 1, 2022

This validation may need to be updated to include the fix for #1457 whenever that is implemented.

@marmijo
Copy link
Contributor Author

marmijo commented Sep 6, 2022

I updated this PR to add validation to versions of Ignition in config

config/v3_4_experimental/types/config.go Show resolved Hide resolved
config/v3_4_experimental/types/config.go Outdated Show resolved Hide resolved
config/v3_4_experimental/types/config.go Outdated Show resolved Hide resolved
config/v3_4_experimental/types/config.go Outdated Show resolved Hide resolved
config/v3_4_experimental/types/config.go Outdated Show resolved Hide resolved
config/shared/errors/errors.go Outdated Show resolved Hide resolved
docs/release-notes.md Outdated Show resolved Hide resolved
config/v3_4_experimental/types/config_test.go Outdated Show resolved Hide resolved
config/v3_4_experimental/types/config_test.go Outdated Show resolved Hide resolved
Copy link
Contributor Author

@marmijo marmijo left a comment

Choose a reason for hiding this comment

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

These errors in config_test.go are what I saw locally. I'll add back in the other lines explicitly setting these other members to be empty. https://github.com/coreos/ignition/actions/runs/3191845930/jobs/5208635971#step:5:30

@marmijo marmijo force-pushed the systemd_storage_conflict branch 2 times, most recently from ffd0196 to 66ed4ce Compare October 5, 2022 18:51
@marmijo marmijo requested a review from bgilbert October 5, 2022 18:59
Copy link
Contributor

@bgilbert bgilbert left a comment

Choose a reason for hiding this comment

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

Some small fixes, but this generally looks good. 👍

Please don't include substantive changes and a rebase in the same force-push; it makes the interdiff hard to read. We generally try to rebase, force-push, and then make substantive changes (or the other way around).

config/v3_4_experimental/types/config.go Show resolved Hide resolved
config/v3_4_experimental/types/config_test.go Outdated Show resolved Hide resolved
config/v3_0/types/config.go Outdated Show resolved Hide resolved
config/v3_0/types/config.go Outdated Show resolved Hide resolved
config/v3_0/types/config.go Outdated Show resolved Hide resolved
config/v3_4_experimental/types/config.go Outdated Show resolved Hide resolved
config/v3_0/types/config_test.go Show resolved Hide resolved
config/v3_0/types/config_test.go Outdated Show resolved Hide resolved
config/v3_0/types/config_test.go Outdated Show resolved Hide resolved
config/v3_0/types/config.go Outdated Show resolved Hide resolved
@marmijo
Copy link
Contributor Author

marmijo commented Nov 8, 2022

Please don't include substantive changes and a rebase in the same force-push; it makes the interdiff hard to read. We generally try to rebase, force-push, and then make substantive changes (or the other way around).

I didnt know think about how it would affect the PR in that way. Thanks for the feedback!

config/v3_0/types/config.go Outdated Show resolved Hide resolved
config/v3_0/types/config.go Outdated Show resolved Hide resolved
config/v3_0/types/config_test.go Outdated Show resolved Hide resolved
config/v3_1/types/config_test.go Outdated Show resolved Hide resolved
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.

Ignition should fail validation if files/links/dirs conflict with systemd units or drop ins
3 participants