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

Respect aws_profile from Keygroup Config #1049

Merged
merged 3 commits into from
Oct 13, 2023
Merged

Conversation

Kouzukii
Copy link
Contributor

@Kouzukii Kouzukii commented May 2, 2022

A KMS entry in a creation_rule keygroup supports setting aws_profile, but the value is not passed into the KMS MasterKey.

Fixes: #1093

@abeluck
Copy link

abeluck commented Jun 30, 2022

I can confirm that this patch fixes the bug that prevents aws_profile from working when defined in a keygroup.

sample .sops.yml

---
creation_rules:
  - key_groups:
      - kms:
        - arn: arn:aws:kms:eu-west-1......
          aws_profile: my-profile

With latest master branch or release version 2.7.3

$ sops --verbose test2.sops.yml
[AWSKMS]         INFO[0006] Encryption failed                             arn="arn:aws:kms:eu-west-1:...redacted..."
Error encrypting the data key with one or more master keys: [failed to encrypt new data key with master key "arn:aws:kms:eu-west-1:...redacted...": Failed to call KMS encryption service: NoCredentialProviders: no valid providers in chain. Deprecated.
        For verbose messaging see aws.Config.CredentialsChainVerboseErrors]

With this patch applied

$ /workspace/go/bin/sops --verbose test2.sops.yml
[AWSKMS]         INFO[0000] Encryption succeeded                          arn="arn:aws:kms:eu-west-1:...redacted..."
[CMD]    INFO[0002] File written successfully   

@WillerWasTaken
Copy link

Oh this was the issue I had here. Would love to see it merged :)

@jgournet
Copy link

Any chance somehow can review/approve ? this would be very useful

@ajvb
Copy link
Contributor

ajvb commented Sep 1, 2022

@Kouzukii can you change this to be against develop instead of master?

@Kouzukii Kouzukii changed the base branch from master to develop September 1, 2022 20:39
@Kouzukii Kouzukii force-pushed the master branch 2 times, most recently from e1958ce to 2714ec1 Compare September 1, 2022 21:17
@Kouzukii
Copy link
Contributor Author

Kouzukii commented Sep 1, 2022

@ajvb done

@jgournet
Copy link

almost there :) can someone review it please ?

@jgournet
Copy link

@ajvb : Could you review this PR please ?

@enchorb
Copy link

enchorb commented Nov 25, 2022

bump

@Kouzukii Kouzukii requested a review from rafzei January 10, 2023 13:58
@enchorb enchorb mentioned this pull request Jan 31, 2023
Copy link
Member

@hiddeco hiddeco left a comment

Choose a reason for hiding this comment

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

In addition to my comment, it would be great if you could rebase and sign-off your commit as we introduced a DCO.

kms/keysource.go Outdated Show resolved Hide resolved
Copy link
Member

@hiddeco hiddeco left a comment

Choose a reason for hiding this comment

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

Addressed some tiny nits I didn't want you to bother with. Thank you very much @Kouzukii! 🍒

@hiddeco hiddeco merged commit c47a29b into getsops:main Oct 13, 2023
@hiddeco hiddeco added this to the v3.9.0 milestone Oct 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Configuring sops to use different AWS KMS keys in different accounts
8 participants