-
Notifications
You must be signed in to change notification settings - Fork 4.3k
feat(opensearch): add logic to only log specific field when less verbosity is needed for opensearch access policy custom resource #34701
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
Conversation
This reverts commit 08ffaa46d93fc896e7293c964b4f7c9381f3d25a.
Adds a clarification note that these properties are not strictly enforced at runtime, but are still required nonetheless. ### Issue #34645 Fixes #34645. ### Reason for this change [ECS docs](https://docs.aws.amazon.com/AmazonECS/latest/developerguide/task_definition_parameters.html#task_size) was recently updated to state that the CPU/Memory property is not enforced on Windows platforms, but are still required nonetheless. ### Description of changes Add note to the CPU and memory property of Fargate task definitions. ### Describe any new or updated permissions being added No permissions added or updated. ### Description of how you validated changes No tests needed as this is a documentation change. ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
### Issue # (if applicable) Closes #31755. ### Reason for this change The dev dependency is outdated and creates issue for users wanting to use some of the aws-sdk packages in custom resources. ### Description of changes <!-- What code changes did you make? Have you made any important design decisions? What AWS use cases does this change enable? To enable the use cases, which AWS service features are utilized? --> The goal of this change is to update the outdated `aws-sdk-js-codemod` package to the latest available version (bumping the major version from 0.x.x to 2.x.x). The following changes have been made: 1. Updated the `aws-sdk-js-codemod` 1. Ran scripts/update-sdkv3-parameters-model.sh 1. Fixed incorrect IAM prefix generated for CloudWatch actions, see #33078 and doc: https://docs.aws.amazon.com/AmazonCloudWatch/latest/monitoring/permissions-reference-cw.html 1. Set the feature flag `logApiResponseDataPropertyTrueDefault` to false (which is the default value) for the tests that needed a snapshot update, as there is a bug that causes the snapshots to be outdated: #30583 1. Ran the tests with `--update-on-failed` to update snasphots 1. Reverted the feature flag changes. ### Description of how you validated changes Ran the integration tests and updated the snapshots. - `framework-integ/test/aws-elasticloadbalancingv2/test/integ.alb.oidc.ts`: needs a valid domain so successfully deployed it on my personal account, then regenerated the snapshots with `--dry-run --force` - `packages/@aws-cdk-testing/framework-integ/test/aws-elasticloadbalancingv2-actions/test/integ.cognito.js.snapshot/integ-cognito.template.json`: needs a valid domain so successfully deployed it on my personal account, then regenerated the snapshots with `--dry-run --force` ### Checklist - [X] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
…eeded for opensearch access policy custom resource
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(This review is outdated)
|
Asking for Adding integration test for this stand-alone custom resource without the |
packages/aws-cdk-lib/aws-opensearchservice/lib/opensearch-access-policy.ts
Outdated
Show resolved
Hide resolved
packages/aws-cdk-lib/aws-opensearchservice/lib/opensearch-access-policy.ts
Show resolved
Hide resolved
packages/aws-cdk-lib/aws-opensearchservice/test/opensearch-access-policy.test.ts
Outdated
Show resolved
Hide resolved
packages/aws-cdk-lib/aws-opensearchservice/lib/opensearch-access-policy.ts
Outdated
Show resolved
Hide resolved
packages/aws-cdk-lib/aws-opensearchservice/lib/opensearch-access-policy.ts
Show resolved
Hide resolved
… search access policy custom resource
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.
|
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
|
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
|
Comments on closed issues and PRs are hard for our team to see. |
Issue # (if applicable)
Closes #29093
Reason for this change
Opensearch access policy defined via
OpenSearchAccessPolicy(custom-resource) return failures in case of large policy documents, even if the policy change is successfulIssue comes for the CFN limit of 4k on the response size
Description of changes
Added an optional parameter
verboseOutputinOpenSearchAccessPolicyPropsto allow users of the custom resource to optionally toggle on/off the verbose option : On turning itfalseonly["DomainConfig.AccessPolicies.Status.State", "DomainConfig.AccessPolicies.Status.UpdateVersion"]are shownNOTE : Default behavior of verbose output is retained
Describe any new or updated permissions being added
NONE
Description of how you validated changes
Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license