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

feat(dynamodb): add ability to enable contributor insights on Global Secondary Index #22700

Closed
wants to merge 10 commits into from

Conversation

DeepController
Copy link

@DeepController DeepController commented Oct 29, 2022

Resolves #15671

Contributor:ynwangn@amazon.com


All Submissions:

Adding new Unconventional Dependencies:

  • This PR adds new unconventional dependencies following the process described here

New Features

  • Have you added the new feature to an integration test?
    • Did you use yarn integ to deploy the infrastructure and generate the snapshot (i.e. yarn integ without --dry-run)?

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

@gitpod-io
Copy link

gitpod-io bot commented Oct 29, 2022

@github-actions github-actions bot added beginning-contributor [Pilot] contributed between 0-2 PRs to the CDK effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p2 labels Oct 29, 2022
@aws-cdk-automation aws-cdk-automation requested a review from a team October 29, 2022 06:48
Copy link
Collaborator

@aws-cdk-automation aws-cdk-automation left a comment

Choose a reason for hiding this comment

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

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

@DeepController
Copy link
Author

PR Linter failed because I did not make any change to the README file. After inspecting the README file, I don't see a need to update the file. Please consider exemption. Thanks!

*
* @default false
*/
readonly contributorInsightsEnabled?: boolean;
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this the correct scope for adding this? It seems like a prop that should be available on more than just a global secondary index.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for noting this! I will move this option into SchemaOptions

@aws-cdk-automation
Copy link
Collaborator

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

aws-cdk-automation and others added 10 commits November 4, 2022 09:44
…rtificate (aws#22509)

Currently, the IAM OIDC Provider is retrieving leaf certificates for a given url. The validity for these certificates is not that long. This can cause an outage for the customer since they might not be aware of when the certificate is going to expire. We have seen an [outage](aws#8607) in EKS due to this issue. 

This change will help retrieving root certificates instead of leaf certificates. The validity of root certificate is much more than the leaf certificates. I am also adding validations for the certificate and also informing the customer if there retrieved certificate is going to expire within six months when they do a new deployment.

Fixes aws#8607

Signed-off-by: Vinayak Kukreja <vinakuk@amazon.com>

### All Submissions:

* [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [x] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [x] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
…he Queue, not the Rule (aws#22766)

When restricting access to encrypted queues, we should use the account of the Rule, instead of the account of the Queue itself.

----

### All Submissions:

* [ ] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
…erty (aws#22779)

Addressing the awslint rule that requires that resources must represent all cloudformation attributes as attribute properties.

----

### All Submissions:

* [ ] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Allowing breakpoints in CodeBuild builds using SSM session manager was already something users could do, but add a property to make the feature slightly more convenient to use, and advertise it better.


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
…22781)

Version 2.48.0 introduced a flag to enable/disable parallelism when
building and publishing assets (see aws#22579 ). The flag is supposed to
be set via either environment variable (`CDK_ASSET_PARALLELISM`), command
line argument (`-[no]-asset-parallelism`), or the `cdk.json` file. However,
the last method does not work. We are not passing the option from that
file properly.

----

### All Submissions:

* [X] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
…sis) (aws#22780)

The `TreeMetadata` resource was being created as part of the `App` constructor, which meant that you could not add any context to the App node (you must add context _before_ any constructs are added). This moves the creation of `TreeMetadata` to be injected during `synthesis` similar to how we currently inject the metadata resource.

A side effect of this change is that we will now include tree metadata for resources that are added in `Stack.synthesize()` methods. The [synthesizeTree](https://github.com/aws/aws-cdk/blob/95e9e0f54a1dfb628d5146bd9b69c292547cf090/packages/@aws-cdk/core/lib/private/synthesis.ts#L193-L210) function visits nodes in the order that they were added to the tree. Previously the `TreeMetadata` was the first node so the TreeMetadata was generated first. Then if any Stacks would call `Stack.synthesize()` and add additional constructs (e.g. `BootstrapVersion`) these would not be added to the tree.

re aws#22749

----

### All Submissions:

* [ ] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

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

### All Submissions:

* [ ] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Require feature flag information to be more structured, and generate a Markdown report automatically that people can refer to if they need to review their feature flag setup.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@DeepController DeepController reopened this Nov 6, 2022
@aws-cdk-automation
Copy link
Collaborator

The pull request linter fails with the following errors:

❌ CLI code has changed. A maintainer must run the code through the testing pipeline (git fetch origin pull/22700/head && git push -f origin FETCH_HEAD:test-main-pipeline), then add the 'pr-linter/cli-integ-tested' label when the pipeline succeeds.

PRs must pass status checks before we can provide a meaningful review.

1 similar comment
@aws-cdk-automation
Copy link
Collaborator

The pull request linter fails with the following errors:

❌ CLI code has changed. A maintainer must run the code through the testing pipeline (git fetch origin pull/22700/head && git push -f origin FETCH_HEAD:test-main-pipeline), then add the 'pr-linter/cli-integ-tested' label when the pipeline succeeds.

PRs must pass status checks before we can provide a meaningful review.

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
  • Commit ID: 1916699
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

This PR has been in the CHANGES REQUESTED state for 3 weeks, and looks abandoned. To keep this PR from being closed, please continue work on it. If not, it will automatically be closed in a week.

@aws-cdk-automation
Copy link
Collaborator

This PR has been deemed to be abandoned, and will be automatically closed. Please create a new PR for these changes if you think this decision has been made in error.

@aws-cdk-automation aws-cdk-automation added the closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. label Nov 30, 2022
@aws-cdk-automation
Copy link
Collaborator

The pull request linter fails with the following errors:

❌ CLI code has changed. A maintainer must run the code through the testing pipeline (git fetch origin pull/22700/head && git push -f origin FETCH_HEAD:test-main-pipeline), then add the 'pr-linter/cli-integ-tested' label when the pipeline succeeds.

PRs must pass status checks before we can provide a meaningful review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beginning-contributor [Pilot] contributed between 0-2 PRs to the CDK closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

(aws-dynamodb): Support ContributorInsightsSpecification for Global Secondary Index
8 participants