sops icon indicating copy to clipboard operation
sops copied to clipboard

Allow aws profile setting from metadata to be overridden.

Open martelli opened this issue 1 year ago • 4 comments

When decrypting, sops uses the AWS profile setting stored in the encrypted file metadata. This is a problem as the profile can change from user to user.

This change will allow the AWS profile metadata setting to be overridden by the '--aws-profile' flag and the AWS_PROFILE environment variable, in that order of precedence. The metadata value is used as a last resort only.

martelli avatar Oct 24 '24 14:10 martelli

One is that this looks like a potential breaking change to me, since a lot of AWS users might have AWS_PROFILE already set (it's an official AWS environment variable), and for them SOPS might suddenly behave differently and stop decrypting files that it was able to decrypt before. Or is this not possible for some reason? (I really don't know.)

First, thanks for the comments and the time to review it. :)

Yeah, I'm also no sure how people usually go about it, but it was unexpected for me to have a profile be determined by the encrypted file. My decryption was failing and it took me a while to figure out why AWS_PROFILE was not being honored. Turned out I don't have any profile named like in the metadata.

Since AWS_PROFILE is quite standard for AWS tooling, this felt like the natural fix. But I get it might break for some people.

I believe we could split this into a few steps:

  1. Have sops honor the --aws-profile flag (which doesn't happen right now). This should be low impact, as it's a new flag.

If we decide to move towards having AWS_PROFILE be authoritative:

  1. Have a grace period where sops warns users whenever AWS_PROFILE and the metadata value disagree (indicating a future break) and announce a deprecation period.

  2. After the deprecation period, switch to use AWS_PROFILE as default, but still warn users about conflicts with the metadata. Maybe even add to the message an instruction to try using the embedded profile name via --aws-profile.

I'd expect people to support this, as it might make life easier by not having to sync profile names across environments, specially around ci/cd pipelines.

How does it sound?

martelli avatar Oct 28 '24 19:10 martelli

@getsops/maintainers any ideas about ^?

felixfontein avatar Dec 24 '24 15:12 felixfontein

One is that this looks like a potential breaking change to me, since a lot of AWS users might have AWS_PROFILE already set (it's an official AWS environment variable), and for them SOPS might suddenly behave differently and stop decrypting files that it was able to decrypt before. Or is this not possible for some reason? (I really don't know.)

First, thanks for the comments and the time to review it. :)

Yeah, I'm also no sure how people usually go about it, but it was unexpected for me to have a profile be determined by the encrypted file. My decryption was failing and it took me a while to figure out why AWS_PROFILE was not being honored. Turned out I don't have any profile named like in the metadata.

Just 2 cents. For us, 3.8.* seem to behave "correctly" and ignore the profile in the encrypted files, and continue resolve the usual AWS SDK way, via an instance role.

With 3.9, there seems to be a change, where if "profile" is present in the encrypted file, it must exist. If it does not, decrypt fails.

bmtKIA6 avatar Jan 23 '25 02:01 bmtKIA6

Just 2 cents. For us, 3.8.* seem to behave "correctly" and ignore the profile in the encrypted files, and continue resolve the usual AWS SDK way, via an instance role.

With 3.9, there seems to be a change, where if "profile" is present in the encrypted file, it must exist. If it does not, decrypt fails.

Maybe we can add a flag to ignore the embedded profile?

Or even try the embedded and fallback to AWS_PROFILE if the embedded profile is not defined?

My understanding is that the embedded profile should not be enforced, but rather be just a hint. AWS will tell if access is to be granted or not.

martelli avatar Jan 23 '25 12:01 martelli