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

fix: fix policies with overridden table names #3075

Conversation

palpatim
Copy link
Member

@palpatim palpatim commented Dec 13, 2024

Description of changes

Fixes policy resources generated for tables with overridden names. The change is simple: it removes the withoutPolicyUpdates() modifier from the role returned during DDB resolver resource creation. This allows Amplify's call to the AppSync CDK API addDynamoDbDataSource to set up a default policy. That grant API uses the table construct to determine the name, rather than relying on the Amplify naming convention. The default policy is therefore scoped to table & index resources with the appropriate name.

Issue #, if available

#3006

Description of how you validated changes

  • Local testing
  • E2E testing

Checklist

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@palpatim palpatim marked this pull request as ready for review December 14, 2024 00:17
@palpatim palpatim requested a review from a team as a code owner December 14, 2024 00:17
@palpatim palpatim requested a review from atierian December 16, 2024 18:28
@palpatim palpatim merged commit fdcb760 into release-api-plugin-stable Dec 17, 2024
8 checks passed
@palpatim palpatim deleted the palpatim.fix.policies-with-overridden-table-name branch December 17, 2024 18:10
@palpatim palpatim mentioned this pull request Dec 17, 2024
2 tasks

while (isTruncated) {
const params = marker ? { Marker: marker } : {};
const response = await service.listRoles(params).promise();
Copy link
Contributor

Choose a reason for hiding this comment

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

does this need paginated?

Copy link
Member Author

@palpatim palpatim Jan 6, 2025

Choose a reason for hiding this comment

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

Late response to this comment: the listRoles response is paginated, using Marker as the "next token" equivalent. See https://docs.aws.amazon.com/AWSJavaScriptSDK/v3/latest/client/iam/command/ListRolesCommand/

Siqi-Shan added a commit that referenced this pull request Dec 20, 2024
Siqi-Shan added a commit that referenced this pull request Dec 20, 2024
palpatim added a commit that referenced this pull request Jan 8, 2025
palpatim added a commit that referenced this pull request Jan 15, 2025
* Revert "Revert "fix: fix policies with overridden table names (#3075)" (#3088)"

This reverts commit e592dba.

* test: migrate OIDC auth tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants