awslabs / aws-servicebroker

AWS Service Broker
Apache License 2.0
468 stars 132 forks source link

Using assumed role appears to be broken #37

Closed mook-as closed 5 years ago

mook-as commented 5 years ago

Describe the bug The docs says that it's possible to use an assumed role by providing the target_account_id and target_role_name configuration values.

To Reproduce Create a service broker with target_account_id and target_role_name parameters

Expected behavior The resource are created with the given role.

Actual behavior The service creation request is rejected, with The parameter target_account_id is not available.

Environment (please complete the following information):

Additional context AwsBroker.Provision rejects requests with unknown parameters; it does not check nonCfnParams (where the role stuff is).

cesartl commented 5 years ago

Same issue here, the target_role_name is actually not in available in the helm chart. I believe this would be the right place to set it.

vsomayaji commented 5 years ago

@mook-as the templates have been updated to add target_account_id and target_role_name and to not require aws_access_key and aws_secret_key (see this comment from @jaymccon). Can you verify that this resolves your issue?

@jaymccon re: @cesartl's comment, the Helm installer currently allows overriding aws_access_key and aws_secret_key:

https://github.com/awslabs/aws-servicebroker/blob/b0e6ede7e920816da6e08561b736e9a7816e5863/packaging/helm/aws-servicebroker/templates/broker-deployment.yaml#L87-L96

Should we update it to also allow overriding target_account_id and target_role_name?

mook-as commented 5 years ago

I can verify that (at least for the rdsmysql I'm testing with), target_account_id and target_role_name are valid parameters, so assuming roles work.

I feel like having that in the templates (instead of the code) is likely to lead to mismatches as the repo changes along with newer versions and the deployed binary stays behind, but that will be resolved when the generator is removed and the broker reads the source material directly (and this stuff will come back again), so that's no reason to keep this open.

Closing.

cesartl commented 5 years ago

do you mean that target_role_name and target_account_id should be passed to every service instance as opposed to be set once in the broker?

cesartl commented 5 years ago

Also when the service broker starts I see the message Parameter 'target_role_name' not set. Not assuming role. This has nothing to do with service instances yet. If we can't pass the target_role_name parameter via helm, how can else can we set it?

vsomayaji commented 5 years ago

@cesartl I think the Helm template needs to be updated to support setting target_account_id and target_role_name as global overrides. @jaymccon does that sound right?

@cesartl until then, you can set those parameters when provisioning an instance. For example:

svcat provision my-instance-name \
  -n my-app \
  --class my-instance-class \
  --plan prd \
  -p target_account_id=123456789012,target_role_name=aws-service-broker-worker

If target_account_id is the current account ID, it can be omitted (that's the default).

Also, the assume role bit only works at provision/bind time right now. At startup, the service broker will always use either the credentials or the instance profile for its S3 and DynamoDB calls; so, that Parameter 'target_role_name' not set. Not assuming role. message can be ignored.

cesartl commented 5 years ago

Ok thanks @vsomayaji. For some reason my broker is not able to access the S3 bucket and I thought this might help but I'll look for something else:

Listing objects bucket: awsservicebroker region: us-east-1 prefix: templates/latest
failed to list objects, AccessDenied: Access Denied
vsomayaji commented 5 years ago

@cesartl hmm. That call is made using the service broker's credentials, not the target_role_name. Can you confirm that those credentials have these permissions:

              -
                Effect: Allow
                Action: s3:GetObject
                Resource: arn:aws:s3:::awsservicebroker/*
              -
                Effect: Allow
                Action: s3:ListBucket
                Resource: arn:aws:s3:::awsservicebroker