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

flatcar/v1_2_exp: Add clevis support #523

Merged
merged 1 commit into from Apr 15, 2024

Conversation

simoncampion
Copy link

This PR updates flatcar/v1_2_exp to reflect Flatcar can handle disk encryption with clevis, since this commit.

// Return FieldFilters for this spec.
func (c Config) FieldFilters() *cutil.FieldFilters {
return &fieldFilters
return &cutil.FieldFilters{}
Copy link

Choose a reason for hiding this comment

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

Suggested change
return &cutil.FieldFilters{}
return nil

Thanks for the PR! The only thing I would see is that the other translate.go files use nil here

Copy link
Author

Choose a reason for hiding this comment

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

Changed it to nil here. (This also required a tiny change to the test logic, handling the case of FieldFilters() returning nil.)

@simoncampion simoncampion marked this pull request as ready for review April 2, 2024 17:03
Copy link
Contributor

@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.

Awesome, thank you for the update!
The changes lgtm, but could you add a comment in the release notes please :)

Edit: also looks like we might need to re run generate to get updated docs (CI pointed it out).

@simoncampion
Copy link
Author

@prestist Thanks! I added an entry in the release notes and ran ./generate.

Copy link
Contributor

@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 working on this. Overall LGTM!

@prestist prestist merged commit dd549bb into coreos:main Apr 15, 2024
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.

None yet

3 participants