Skip to content

feat: add validation to merged local/inline configs#602

Merged
prestist merged 1 commit intocoreos:mainfrom
Roshan-R:fix/issue-275
May 13, 2025
Merged

feat: add validation to merged local/inline configs#602
prestist merged 1 commit intocoreos:mainfrom
Roshan-R:fix/issue-275

Conversation

@Roshan-R
Copy link
Copy Markdown
Contributor

@Roshan-R Roshan-R commented May 7, 2025

Closes #275

@Roshan-R Roshan-R marked this pull request as draft May 7, 2025 05:47
@Roshan-R Roshan-R marked this pull request as ready for review May 9, 2025 04:53
Copy link
Copy Markdown
Member

@travier travier left a comment

Choose a reason for hiding this comment

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

Mostly LGTM. A couple of small changes.

@travier
Copy link
Copy Markdown
Member

travier commented May 12, 2025

Can you also copy this validation code to all stable specs? See #476 (comment):

This validation isn't FCOS-specific; it applies to all variants. So, it should be in base. And since we're only going to be producing errors for configs that were already invalid (but would be rejected when the machine boots rather than when Butane runs) we should backport it to stable base specs.

@Roshan-R
Copy link
Copy Markdown
Contributor Author

Can you also copy this validation code to all stable specs? See #476 (comment):

This validation isn't FCOS-specific; it applies to all variants. So, it should be in base. And since we're only going to be producing errors for configs that were already invalid (but would be rejected when the machine boots rather than when Butane runs) we should backport it to stable base specs.

I have done the backporting to all the versions except v1 since It does not have this feature

@prestist
Copy link
Copy Markdown
Collaborator

Thank you for working on this; these changes make sense to me, and LGTM, one small nit, not needed but would be nice. The other nit is can we maybe cleanup the commit history a bit, i.e squash redundancies.

Validate Ignition configs that are embedded directly in Butane configs,
either as inline content or local file references.
@Roshan-R
Copy link
Copy Markdown
Contributor Author

Commits have been squashed and all requested changes have been addressed.
Let me know if there's anything else needed

Copy link
Copy Markdown
Collaborator

@prestist prestist left a comment

Choose a reason for hiding this comment

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

Thank you for doing this; it lgtm well done!

@prestist prestist merged commit 0711f27 into coreos:main May 13, 2025
7 checks passed
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.

Validate merged/replaced Ignition configs if they're local/inline

3 participants