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.

Sign up for GitHub

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

add templates for a few magic links #69

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

add templates for a few magic links #69

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 13 additions & 22 deletions link2aws.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ class ARN {
"analyzer": () => `https://${this.region}.${this.console}/access-analyzer/home?region=${this.region}#/analyzer/${this.resource}`,
},
"acm": { // AWS Certificate Manager
"certificate": () => `https://${this.console}/acm/home?region=${this.region}#/?id=${this.resource}`,
"certificate": () => `https://${this.console}/acm/home?region=${this.region}#/certificates/${this.resource}`,
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks. I have verified that this change is correct.

However, you also need to update the unit tests. Currently they are failing due to this change. Search for arn:aws:acm:us-east-1:123456789012:certificate/1f6ee793-4064-4a10-9567-f03875640b35 in https://github.com/link2aws/link2aws.github.io/blob/master/testcases/aws.json - the console URL there is currently outdated.

},
"acm-pca": { // AWS Certificate Manager Private Certificate Authority
"certificate-authority": null,
Expand All @@ -182,7 +182,11 @@ class ARN {
},
},
"apigateway": { // Manage Amazon API Gateway
"": null,
"": () => {
const parts = this.resource.split('/');
Copy link
Contributor

Choose a reason for hiding this comment

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

I understand why you did this, but this is the wrong solution. Actually what has happened is that you uncovered a bug (very nice! Thank you!). The ARN was parsed via the // ...:resource-id code branch in the constructor() function, but should have been parsed via the // ...:resource-type/resource-id (resource-id can contain slashes!) code branch. Instead of splitting here, can you just change > 0 to >= 0 in the the existing code - from this:

        // ...:resource-type/resource-id (resource-id can contain slashes!)
        else if (typeof (tokens[5]) != 'undefined' && tokens[5].indexOf('/') > 0) {

to this:

        // ...:resource-type/resource-id (resource-id can contain slashes!)
        else if (typeof (tokens[5]) != 'undefined' && tokens[5].indexOf('/') >= 0) {

Then you can simply use the "": () => `...`, pattern here.

I have verified that all existing tests continue to pass with that change. So it really is a case that wasn't exercised before.

const myid = parts[2];
Copy link
Contributor

Choose a reason for hiding this comment

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

Why myid? This could be just id, or could be more specific (maybe api_id). But my is just visual clutter that doesn't convey information and should be removed.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you fix all the whitespace errors?

  1. There is trailing whitespace at the end of the const myid = ... line. Please remove it.
  2. The return line is indented with a mix of tabs and spaces. It should be spaces only.
  3. The closing } has the wrong indent level. It should be indented one level less.

return `https://${this.region}.${this.console}/apigateway/main/apis/${myid}/resources?api=${myid}&region=${this.region}`;
},
},
"appconfig": { // AWS AppConfig
"application": null,
Expand Down Expand Up @@ -287,7 +291,7 @@ class ARN {
},
"codebuild": { // AWS CodeBuild
"build": null,
"project": null,
"project": () => `https://${this.region}.${this.console}/codesuite/codebuild/projects/${this.resource}`,
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add unit tests for all the resource types that you are adding support for?

It's easy to do, existing tests are here: https://github.com/link2aws/link2aws.github.io/blob/master/testcases/aws.json

"report": null,
"report-group": null,
},
Expand Down Expand Up @@ -435,6 +439,7 @@ class ARN {
"dedicated-host": null,
"dhcp-options": null,
"elastic-gpu": null,
"eip-allocation": () => `https://${this.region}.${this.console}/ec2/home?region=${this.region}#Addresses:v=3;search=:${this.resource}`,
"fpga-image": null,
"image": () => `https://${this.region}.${this.console}/ec2/home?region=${this.region}#ImageDetails:imageId=${this.resource}`,
"instance": () => `https://${this.region}.${this.console}/ec2/home?region=${this.region}#InstanceDetails:instanceId=${this.resource}`,
Expand Down Expand Up @@ -475,7 +480,7 @@ class ARN {
"vpn-gateway": null,
},
"ecr": { // Amazon Elastic Container Registry
"repository": null,
"repository": () => `https://${this.region}.${this.console}/ecr/repositories/private/${this.account}/${this.resource}`,
},
"ecs": { // Amazon Elastic Container Service
"cluster": () => `https://${this.region}.${this.console}/ecs/v2/clusters/${this.resource}?region=${this.region}`,
Expand All @@ -502,7 +507,7 @@ class ARN {
"application": null,
"applicationversion": null,
"configurationtemplate": null,
"environment": null,
"environment": () => `https://${this.region}.${this.console}/elasticbeanstalk/home?region=${this.region}#/environments`,
"platform": null,
"solutionstack": null,
},
Expand All @@ -526,7 +531,7 @@ class ARN {
"preset": null,
},
"es": { // Amazon Elasticsearch Service
"domain": null,
"domain": () => `https://${this.region}.${this.console}/aos/home?region=${this.region}#opensearch/domains/${this.resource}`,
},
"events": { // Amazon EventBridge
"event-bus": null,
Expand Down Expand Up @@ -579,7 +584,7 @@ class ARN {
"crawler": null,
"database": null,
"devendpoint": null,
"job": null,
"job": () => `https://${this.region}.${this.console}/gluestudio/home?region=${this.region}#/editor/job/${this.resource}/script`,
"mlTransform": null,
"table": null,
"tableVersion": null,
Expand Down Expand Up @@ -870,7 +875,7 @@ class ARN {
"cluster": () => `https://${this.console}/rds/home?region=${this.region}#database:id=${this.resource};is-cluster=true`,
"cluster-endpoint": null,
"cluster-pg": null,
"cluster-snapshot": null,
"cluster-snapshot": () => `https://${this.console}/rds/home?region=${this.region}#db-snapshot:id=${this.resource}`,
"db": () => `https://${this.console}/rds/home?region=${this.region}#database:id=${this.resource}`,
"db-proxy": null,
"es": null,
Expand Down Expand Up @@ -1129,17 +1134,3 @@ class ARN {
}
}
}

// Running as command line script? (not in browser, and not as library)
/* istanbul ignore if */
if (typeof (require) !== 'undefined' && require.main === module) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are you removing all this code?

for (let i = 2; i < process.argv.length; i++) {
try {
console.log(new ARN(process.argv[i]).consoleLink);
} catch (e) {
console.error(e);
}
}
}

exports.ARN = ARN;
Loading