feat(IamPolicyChecks): Refactor code(constants, functions, errors), move from DevSettings to Experiments, change S3Client #4964
Conversation
…ns), move to experiments, change S3 client
| * Adapted from | ||
| * @see https://github.com/frantz/amazon-s3-uri/ | ||
| */ | ||
| export function parseS3Uri(uri: string): [region: string, bucket: string, key: string] { |
There was a problem hiding this comment.
It's very important to avoid dependencies, but I'm not sure I understand the motivation for "inlining" this particular case. Was it pulling in outdated transitive packages, or some other reason?
There was a problem hiding this comment.
I don't think it was pulling in anything transitive. @hayemaxi suggested that we move away from the dependency just as a general standard I believe.
| @@ -2017,7 +2022,7 @@ | |||
| "command": "aws.accessanalyzer.iamPolicyChecks", | |||
| "title": "%AWS.command.accessanalyzer.iamPolicyChecks%", | |||
| "category": "%AWS.title%", | |||
| "enablement": "aws.iamPolicyChecks.enabled" | |||
| "enablement": "config.aws.experiments.iamPolicyChecks" | |||
There was a problem hiding this comment.
Do we plan to create another PR to enable this by default before the launch and release that version through the toolkits pipeline?
There was a problem hiding this comment.
I am assuming that is the process, but this will now make it available through Experiments, which is a checkbox in the settings.
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| export abstract class IamPolicyChecksConstants { |
There was a problem hiding this comment.
We should review these error messages with the doc writer
| if (parsedUri.protocol === 's3:') { | ||
| bucket = parsedUri.host ?? undefined | ||
| if (!bucket) { | ||
| throw new Error(`Invalid S3 URI: no bucket: ${uri}`) |
There was a problem hiding this comment.
Why do we want to lint the URI given? Can't we rely on S3 for that?
There was a problem hiding this comment.
S3 does not support GetObject using URI out of the box. The idea of this is to get the region, bucket, and key from the URI and use those parameters to perform GetObject.
Refactor code by moving static strings to a constants file, creating help functions, and updating error messages.
Moving from DevSettings to Experiments to allow access.
S3Client changed to use
DefaultS3Clientand adapted URI parser dependency to a helper function.License
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.