feat: introduce "node.addr" (deprecates "uniqueId") #314
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The `uniqueId` property has a few issues: 1. It uses MD5 which is not FIPS-compliant (fixes #273) 2. They have variable length of up to 255 chars which may not be suitable in certain domains (e.g. k8s label names are limited to 63 characters). 3. The human-readable portion of `uniqueId` is sometimes confusing and in many cases does not offer sufficient value. 4. Their charset might be restrictive or too broad for certain domains. To that end, we introduce `node.addr` as an alternative to `uniqueId`. It returns a 42 character hexadecimal, lowercase and opaque address for a construct which is unique within the tree (app). The address is calculated using SHA-1 which is FIPS-compliant. For example: c83a2846e506bcc5f10682b564084bca2d275709ee Similar to `uniqueId`, `addr` intentionally skips any path components called `default` (case insensitive) to allow refactoring of construct trees which preserve names within the tree.
eladb
changed the title
node.addr (deprecates node.uniqueId)
Closed
|
Isn't it better to allow customizing the addressing strategy at the root, in some way? I don't see the point in deprecating How does this change interact with people using AWS CDK EKS+cdk8s in the same program? What if they want to FIPS-ify their setup similarly in that scenario (whatever that means)? |
|
The idea is that resource naming strategy is left to the domain, but that strategy can use As for deprecating uniqueId: my plan is that in 2.0 we will move the implementation to the AWS CDK to support legacy usage but remove from constructs 10.x and future usage in AWS CDK. Makes sense? |
rix0rrr
approved these changes
Oct 26, 2020
eladb
pushed a commit
to aws/aws-cdk
that referenced
this pull request
Oct 28, 2020
The property `node.uniqueId` is used throughout our codebase to generate unique names. We are planning to deprecate this API in constructs 10.x (and CDK 2.0) in favor of a `node.addr` (see aws/constructs#314). In most cases, these generated names cannot be changed because they will incur breaking changes to deployments. So we have to continue to use the "legacy" unique ID in those cases (new cases should use `addr`). To that end, we introduce a utility `Legacy.uniqueId()` which uses the legacy algorithm and the code base was migrated to use it instead of `node.uniqueId` which will go away soon.
eladb
pushed a commit
to aws/aws-cdk
that referenced
this pull request
Oct 29, 2020
The property `node.uniqueId` is used throughout our codebase to generate unique names. We are planning to deprecate this API in constructs 10.x (and CDK 2.0) in favor of a `node.addr` (see aws/constructs#314). In most cases, these generated names cannot be changed because they will incur breaking changes to deployments. So we have to continue to use the "legacy" unique ID in those cases (new cases should use `addr`). To that end, we introduce a utility `Legacy.uniqueId()` which uses the legacy algorithm and the code base was migrated to use it instead of `node.uniqueId` which will go away soon.
mergify bot
pushed a commit
to aws/aws-cdk
that referenced
this pull request
Nov 3, 2020
…Id" (#11166) The property `node.uniqueId` is used throughout our codebase to generate unique names. We are planning to deprecate this API in constructs 10.x (and CDK 2.0) in favor of a `node.addr` (see aws/constructs#314). In most cases, these generated names cannot be changed because they will incur breaking changes to deployments. So we have to continue to use the "legacy" unique ID in those cases (new cases should use `addr`). To that end, we introduce a utility `Legacy.uniqueId()` which uses the legacy algorithm and the code base was migrated to use it instead of `node.uniqueId` which will go away soon. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
eladb
pushed a commit
to cdk8s-team/cdk8s
that referenced
this pull request
Nov 18, 2020
The CDK8s name generator (`Names.toLabelValue()` and `Names.toDnsLabel()`) have used sha256, which cannot be used in environments that are FIPS compliant. To fix this, we are now using the [recently introduced](aws/constructs#314) `Node.of(construct).addr` as the hash postfix of generated names. Fixes #334 BREAKING CHANGE: CAUTION! Auto-generated resource names will change with this release. Resource names in manifests synthesized by a previous version of the CDK8s will be invalidated. Deploying new manifests will cause **resources to be replaced**. Temporarily, you can opt to use the legacy hashing mechanism by setting the environment variable `CDK8S_LEGACY_HASH=1`. * **core:** `Names.toDnsLabel()` now accepts a construct scope instead of a string path, and a set of options instead of `maxLen`. * **core:** `Names.toLabelValue()` now accepts a construct scope instead of a string path, and a set of options instead of `maxLen`.
mergify bot
pushed a commit
to cdk8s-team/cdk8s
that referenced
this pull request
Nov 19, 2020
The CDK8s name generator (`Names.toLabelValue()` and `Names.toDnsLabel()`) have used sha256, which cannot be used in environments that are FIPS compliant. To fix this, we are now using the [recently introduced](aws/constructs#314) `Node.of(construct).addr` as the hash postfix of generated names. Fixes #334 BREAKING CHANGE: CAUTION! Auto-generated resource names will change with this release. Resource names in manifests synthesized by a previous version of the CDK8s will be invalidated. Deploying new manifests will cause **resources to be replaced**. Temporarily, you can opt to use the legacy hashing mechanism by setting the environment variable `CDK8S_LEGACY_HASH=1`. * **lib:** `Names.toDnsLabel()` now accepts a construct scope instead of a string path, and a set of options instead of `maxLen`. * **lib:** `Names.toLabelValue()` now accepts a construct scope instead of a string path, and a set of options instead of `maxLen`. *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
|
what about the node.addr at the very top of the tree for the cdk.App() import * as cdk from 'aws-cdk-lib'; const app1 = new cdk.App(); const app2 = new cdk.App(); Can I safely assume that this addr of the App itself will always be unique? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The
uniqueIdproperty has a few issues:uniqueIdis sometimes confusing and in many cases does not offer sufficient value.To that end, we introduce
node.addras an alternative touniqueId. It returns a 42 character hexadecimal, lowercase and opaque address for a construct which is unique within the tree (app). The address is calculated using SHA-1 which is FIPS-compliant.For example:
Similar to
uniqueId,addrintentionally skips any path components calledDefaultto allow refactoring of construct trees which preserve names within the tree.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license