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

[stepfunctions-tasks] SageMakerUpdateEndpoint creates wrong permissions #11594

Closed
cmwilhelm opened this issue Nov 19, 2020 · 4 comments · Fixed by #13170
Closed

[stepfunctions-tasks] SageMakerUpdateEndpoint creates wrong permissions #11594

cmwilhelm opened this issue Nov 19, 2020 · 4 comments · Fixed by #13170
Assignees
Labels
@aws-cdk/aws-stepfunctions-tasks bug This issue is a bug. effort/small Small work item – less than a day of effort p1

Comments

@cmwilhelm
Copy link

❓ General Issue

The Question

There appear to be some incorrect permissions generated when using the aws_stepfunctions_tasks.SageMakerUpdateEndpoint construct in a state machine
created through CDK.

Specifically, the state machine role is granted a permission for updateEndpoint against
resource "arn:aws:sagemaker:<REGION>:<ACCOUNT_ID>:endpoint/*". However, from
the error messages in my StepFn invocation, it appears the resource target should actually
be endpoint-config/*:

{
  "resourceType": "sagemaker",
  "resource": "updateEndpoint",
  "error": "SageMaker.AmazonSageMakerException",
  "cause": "User: arn:aws:sts::<ACCOUNT_DI>:assumed-role/<STATE_MACHINE_NAME>-StateMachineRole<ROLE_ID> is not authorized to perform: sagemaker:UpdateEndpoint on resource: arn:aws:sagemaker:<REGION>:<ACCOUNT_ID>:endpoint-config/<ENDPOINT_CONFIG_NAME> (Service: AmazonSageMaker; Status Code: 400; Error Code: AccessDeniedException; Request ID: 3c22b7ab-4687-475a-b82c-643a75f21a13; Proxy: null)"
}

Environment

We're currently pinned at CDK core and CDK module versions 1.72, using the python bindings.
However, I checked the release notes and did not see any mention of fixes since then to this stepfn task.

@cmwilhelm cmwilhelm added guidance Question that needs advice or information. needs-triage This issue or PR still needs to be triaged. labels Nov 19, 2020
@SomayaB SomayaB changed the title aws_stepfunctions_tasks.SageMakerUpdateEndpoint creates wrong permissions [stepfunctions-tasks] SageMakerUpdateEndpoint creates wrong permissions Nov 20, 2020
@NGL321
Copy link
Contributor

NGL321 commented Dec 16, 2020

From what I can tell from the SageMaker docs, they indicate the proper policies for CreateEndpoint, but do not indicate for UpdateEndpoint. So I think it is fair to say that the policies for UpdateEndpoint should match those for CreateEndpoint.

At the moment, you are correct and update-endpoint only adds policy for endpoint/* not endpoint-config/* as docs would indicate:

private makePolicyStatements(): iam.PolicyStatement[] {
const stack = cdk.Stack.of(this);
// https://docs.aws.amazon.com/sagemaker/latest/dg/api-permissions-reference.html
return [
new iam.PolicyStatement({
actions: ['sagemaker:updateEndpoint'],
resources: [
stack.formatArn({
service: 'sagemaker',
resource: 'endpoint',
// If the endpoint name comes from input, we cannot target the policy to a particular ARN prefix reliably.
// SageMaker uses lowercase for resource name in the arn
resourceName: sfn.JsonPath.isEncodedJsonPath(this.props.endpointName) ? '*' : `${this.props.endpointName.toLowerCase()}`,
}),
],
}),
];
}

I can confirm this bug, and it should only be a 1 line fix!

😸 😷

@NGL321 NGL321 added bug This issue is a bug. effort/small Small work item – less than a day of effort p1 and removed guidance Question that needs advice or information. needs-triage This issue or PR still needs to be triaged. labels Dec 16, 2020
@zxkane
Copy link
Contributor

zxkane commented Feb 10, 2021

Any plan for fixing it or requiring someone contributing PR?

@shivlaks
Copy link
Contributor

@zxkane happy to review it if you want to submit one! it's not currently on my radar but I can pick it up if nobody's interested 😅

@mergify mergify bot closed this as completed in #13170 Mar 3, 2021
mergify bot pushed a commit that referenced this issue Mar 3, 2021
…sions (#13170)

closes #11594

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions
Copy link

github-actions bot commented Mar 3, 2021

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

cornerwings pushed a commit to cornerwings/aws-cdk that referenced this issue Mar 8, 2021
…sions (aws#13170)

closes aws#11594

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-stepfunctions-tasks bug This issue is a bug. effort/small Small work item – less than a day of effort p1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants