Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

fix: condition on the creation of the kms key #1286

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

Open
schniber wants to merge 6 commits into cattle-ops:main
base: main
Choose a base branch
Loading
from schniber:main

Conversation

Copy link

@schniber schniber commented Jun 6, 2025

Description

this PR fixes #1278

Note: The whole PR is used as commit message.

Migrations required

No

Verification

I did deploy this module with both options, i.e. enable_managed_kms_key = true and enable_managed_kms_key = false to validate the fix

meniluca reacted with thumbs up emoji
Copy link
Contributor

github-actions bot commented Jun 6, 2025

Hey @schniber! 👋

Thank you for your contribution to the project. Please refer to the contribution rules for a quick overview of the process.

Make sure that this PR clearly explains:

  • the problem being solved
  • the best way a reviewer and you can test your changes

With submitting this PR you confirm that you hold the rights of the code added and agree that it will published under this LICENSE.

The following ChatOps commands are supported:

  • /help: notifies a maintainer to help you out

Simply add a comment with the command in the first line. If you need to pass more information, separate it with a blank line from the command.

This message was generated automatically. You are welcome to improve it.

Copy link
Author

schniber commented Jun 6, 2025

@npalm : Hey, is it possible to peer review this ?
Happy to iterate based on your inputs.

Bests.

Copy link
Author

Hey @kayman-mk,

Any chance of approving this after your review ?

Thanks a lot.

Bests

Copy link

We are also impacted by #1278 and looking forward to updating the runner to the latest version.

schniber reacted with thumbs up emoji

Copy link
Collaborator

kayman-mk commented Jul 2, 2025
edited
Loading

This change is now incompatible for those guys using a customer managed key. The module can be updated without setting the new variable to true resulting in an outage, I guess or at least using the wrong keys to encrypt the resources.

Not sure how this can be solved.

Copy link
Author

Hey @kayman-mk I tested this PR with both options and it works.

Can you please elaborate a bit more on your last comment ?

Thanks a lot !

Bests.

Copy link
Contributor

This PR is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 15 days.

@github-actions github-actions bot added the stale Issue/PR is stale and closed automatically label Sep 10, 2025
@kayman-mk kayman-mk added work-in-progress Issue/PR is worked, should not become stale and removed stale Issue/PR is stale and closed automatically labels Sep 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@npalm npalm Awaiting requested review from npalm npalm is a code owner

@kayman-mk kayman-mk Awaiting requested review from kayman-mk kayman-mk is a code owner

Assignees

No one assigned

Labels

work-in-progress Issue/PR is worked, should not become stale

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Plan fails if KMS key is being created at the same time as the module

AltStyle によって変換されたページ (->オリジナル) /