-
Notifications
You must be signed in to change notification settings - Fork 16
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
Add autonaming overlay #1828
Add autonaming overlay #1828
Changes from 4 commits
81b7f18
ae72eb2
53a8940
549a72d
e727ba1
946b77d
1ed7447
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
{ | ||
"Resources": { | ||
"AWS::IAM::Role": { | ||
"RoleName": { | ||
"minLength": 1, | ||
"maxLength": 64 | ||
} | ||
}, | ||
"AWS::Lambda::Function": { | ||
"FunctionName": { | ||
"minLength": 1, | ||
"maxLength": 140 | ||
} | ||
}, | ||
"AWS::Lambda::Alias": { | ||
"Name": { | ||
"minLength": 1, | ||
"maxLength": 128 | ||
} | ||
}, | ||
"AWS::ElasticLoadBalancingV2::TargetGroup": { | ||
"Name": { | ||
"minLength": 1, | ||
"maxLength": 32 | ||
} | ||
}, | ||
"AWS::ElasticLoadBalancingV2::LoadBalancer": { | ||
"Name": { | ||
"minLength": 1, | ||
"maxLength": 32 | ||
} | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
/bin/ | ||
/node_modules/ |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
name: aws-native-autonaming-overlay | ||
runtime: nodejs | ||
description: An example program to test autoname overlay |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
import * as aws from "@pulumi/aws-native"; | ||
|
||
new aws.iam.Role("myRole".repeat(11), { | ||
assumeRolePolicyDocument: { | ||
Version: "2012-10-17", | ||
Statement: [ | ||
{ | ||
Effect: "Allow", | ||
Principal: { | ||
Service: "lambda.amazonaws.com", | ||
}, | ||
Action: "sts:AssumeRole", | ||
}, | ||
], | ||
}, | ||
}); |
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
{ | ||
"name": "aws-native-naming-conventions", | ||
"main": "index.ts", | ||
"devDependencies": { | ||
"@types/node": "^16" | ||
}, | ||
"dependencies": { | ||
"@pulumi/pulumi": "^3.74.0", | ||
"@pulumi/aws-native": "^0.8.0" | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
{ | ||
"compilerOptions": { | ||
"strict": true, | ||
"outDir": "bin", | ||
"target": "es2016", | ||
"module": "commonjs", | ||
"moduleResolution": "node", | ||
"sourceMap": true, | ||
"experimentalDecorators": true, | ||
"pretty": true, | ||
"noFallthroughCasesInSwitch": true, | ||
"noImplicitReturns": true, | ||
"forceConsistentCasingInFileNames": true | ||
}, | ||
"files": [ | ||
"index.ts" | ||
] | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,7 @@ | |
package examples | ||
|
||
import ( | ||
"bytes" | ||
"path/filepath" | ||
"testing" | ||
|
||
|
@@ -141,6 +142,21 @@ func TestNamingConventions(t *testing.T) { | |
integration.ProgramTest(t, &test) | ||
} | ||
|
||
func TestAutoNamingOverlay(t *testing.T) { | ||
var buf bytes.Buffer | ||
test := getJSBaseOptions(t). | ||
With(integration.ProgramTestOptions{ | ||
Dir: filepath.Join(getCwd(t), "autonaming-overlay"), | ||
Stderr: &buf, | ||
ExpectFailure: true, | ||
ExtraRuntimeValidation: func(t *testing.T, stackInfo integration.RuntimeValidationStackInfo) { | ||
assert.Contains(t, buf.String(), "is too large to fix max length constraint of 64") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Out of scope, but shouldn't the error message here be "... too large to fit max length constraint". Maybe it's a non-native speaker issue on my end. I can cut a PR if we think this is wrong There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah I think it was just a typo |
||
}, | ||
}) | ||
|
||
integration.ProgramTest(t, &test) | ||
} | ||
|
||
func TestParallelTs(t *testing.T) { | ||
test := getJSBaseOptions(t). | ||
With(integration.ProgramTestOptions{ | ||
|
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.
I think this one is tricky. For auto naming we want 64 because the max length is only 140 if you use a ARN/partial ARN. Otherwise it's 64.
Does the maxLength apply to auto naming only or also for validating the name property if it's set?
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.
But to be honest. I have no clue why somebody would use the ARN or partial ARN when creating a function (or why Lambda supports that)
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.
Wow good catch! We want 64 here since this will only be used in creating an auto name.