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.

Sign up for GitHub

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

Jump to bottom

iam_server_id_header_value not persisted after updating AWS client config #3004

Closed
mp911de opened this issue Jul 13, 2017 · 5 comments
Closed

iam_server_id_header_value not persisted after updating AWS client config #3004

mp911de opened this issue Jul 13, 2017 · 5 comments
Milestone
0.8.0

Comments

@mp911de
Copy link
Contributor

mp911de commented Jul 13, 2017

AWS client config value iam_server_id_header_value is not persisted when updating the AWS client config.

Vault version: Vault v0.7.3 ('0b20ae0b9b7a748d607082b1add3663a28e31b68')

$ vault write auth/aws/config/client iam_server_id_header_value=foo secret_key=… access_key=…
Success! Data written to: auth/aws/config/client

$ vault read auth/aws/config/client 
Key                       	Value
---                       	-----
access_key                	…
endpoint                  	
iam_endpoint              
iam_server_id_header_value	
secret_key                	…
sts_endpoint

The iam_server_id_header_value value gets updates as soon as iam_endpoint gets included:

$ vault write auth/aws/config/client iam_server_id_header_value=foo secret_key=… access_key=… iam_endpoint=bar
Success! Data written to: auth/aws/config/client

$ vault read auth/aws/config/client 
Key                       	Value
---                       	-----
access_key                	…
endpoint                  	
iam_endpoint              	bar
iam_server_id_header_value	foo
secret_key                	…
sts_endpoint
@jefferai
Copy link
Member

I looked at the code and it's actually if (almost) any other value is set that value will be saved as well. But there's a note in the code saying that this is specifically the intended behavior (https://github.com/hashicorp/vault/blob/master/builtin/credential/aws/path_config_client.go#L214)

I'll need to reach out and figure out what the intention is there; maybe it's something we can issue a warning about.

@jefferai jefferai added this to the 0.7.4 milestone Jul 13, 2017
@jefferai
Copy link
Member

@joelthompson Looks like that block is your code, can you provide some insight?

@joelthompson
Copy link
Contributor

Yeah, saw this earlier, will submit a PR tonight to fix.

Basically, in this case, we don't want to flush the cached AWS clients, but we do want to write the updated value back to the underlying storage. The existing code has a flag whose meaning is overloaded to both "flush the cached AWS clients" and "write the updated value back to the underlying storage." I was trying to avoid unnecessarily flushing the cached clients, but missed the part where that would also prevent it from getting written to the storage, and hence this bug.

@jefferai
Copy link
Member

Gotcha. I guess it just means un-overloading the flag, either splitting it into a bit field or into two vars?

@joelthompson
Copy link
Contributor

Yeah, I think that's pretty much it.

joelthompson added a commit to joelthompson/vault that referenced this issue Jul 14, 2017
@joelthompson
Properly store iam_server_id_header_value
d3a2e1d 
In auth/aws/config/client, when only the iam_server_id_header_value was
being updated on an existing config, it wouldn't get stored because I
was trying to avoid unnecessarily flushing the cache of AWS clients, and
the flag to not flush the cache also meant that the updated entry didn't
get written back to the storage. This now adds a new flag for when
other changes occur that don't require flushing the cache but do require
getting written to the storage. It also adds a test for this explicitly.

Fixes hashicorp#3004
@joelthompson joelthompson mentioned this issue Jul 14, 2017
Merged
jefferai pushed a commit that referenced this issue Jul 17, 2017
@joelthompson @jefferai
Properly store iam_server_id_header_value (#3014)
9a0c544 
In auth/aws/config/client, when only the iam_server_id_header_value was
being updated on an existing config, it wouldn't get stored because I
was trying to avoid unnecessarily flushing the cache of AWS clients, and
the flag to not flush the cache also meant that the updated entry didn't
get written back to the storage. This now adds a new flag for when
other changes occur that don't require flushing the cache but do require
getting written to the storage. It also adds a test for this explicitly.

Fixes #3004
@jefferai jefferai modified the milestones: 0.7.4, 0.8.0 Jul 24, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.8.0
Development

No branches or pull requests
3 participants
@jefferai @mp911de @joelthompson