Create approve templates #43
Conversation
pauloancheta
force-pushed
the
create-approve-templates
branch
5 times, most recently
from
5d47767 to
7a8cc8a
Compare
pauloancheta
force-pushed
the
create-request-approval
branch
from
dbc018d to
02d8bde
Compare
There was a problem hiding this comment.
Simliar to #41, I don't think you should assume the extension is or should be .yaml. Either use path.extname to discover the extension or do not append an extension to the path (I think its unlikely these paths will be used in a context where the extension is meaningful anyway).
src/approval/index.ts
Outdated
| } | ||
| }); | ||
| console.log(colorizedString); | ||
| console.log("Do these changes look good for you? Add a flag `--approve true` to approve"); |
There was a problem hiding this comment.
Could this be interactive? Prompt for y/n similar to this.
src/approval/index.ts
Outdated
| return 0 | ||
| } | ||
|
|
||
| // create a new pending file |
There was a problem hiding this comment.
This should probably be in the else branch of the conditional above (even though there is a return).
src/approval/index.ts
Outdated
| Bucket: s3Url.bucket, | ||
| Key: s3Url.key.replace(/\.pending$/, "") | ||
| }).promise(); | ||
| logSuccess("Created a new cfn-template.yaml."); |
There was a problem hiding this comment.
I think you should log this and "Updated latest" at debug level.
src/approval/index.ts
Outdated
| export async function approveTemplate(argv: Arguments): Promise<number> { | ||
| await configureAWS(argv.profile, "us-east-1"); | ||
| const s3 = new S3(); | ||
| const s3Url = parseS3HttpUrl(argv.filename); |
There was a problem hiding this comment.
The same as #41 (comment), this is a misuse of parseS3HttpUrl
src/approval/index.ts
Outdated
| .then((template) => template.Body) | ||
| .catch((e) => { | ||
| if (e.code !== "NoSuchKey") { | ||
| return Promise.reject(e); |
pauloancheta
force-pushed
the
create-approve-templates
branch
from
7a8cc8a to
3732c0c
Compare
src/cfn/index.ts
Outdated
| @@ -1515,6 +1516,7 @@ export async function convertStackToIIDY(argv: Arguments): Promise<number> { | |||
| const stackArgs: StackArgs = { | |||
| Template: './cfn-template.yaml', | |||
| StackName: StackNameArg, | |||
| ApprovedTemplateLocation: '', | |||
There was a problem hiding this comment.
Shouldn't the default here be undefined?
src/approval/index.ts
Outdated
| const s3Bucket = s3Url.hostname ? s3Url.hostname : ""; | ||
|
|
||
| const fileName = new Md5().appendStr(cfnTemplate.toString()).end().toString() | ||
| const fullFileName = `${fileName}${path.extname(stackArgs.Template)}.pending` |
There was a problem hiding this comment.
semicolons on this and line 30.
src/approval/index.ts
Outdated
| previouslyApprovedTemplate!.toString(), | ||
| pendingTemplate!.toString(), | ||
| ); | ||
| let colorizedString = "" |
There was a problem hiding this comment.
same in other spots in this file
|
Let's squash the commits prior to merge and then rebase #45. |
jpb
force-pushed
the
create-approve-templates
branch
from
5a85793 to
420abd5
Compare
the command is used as an approval process for the cfn-template.yaml. When a template is uploaded to an s3 bucket through the cmd, it appends the suffix `.pending` to the filename. This then enables the maintainers/admins to review the changes in the cfn-template.yaml before using it for the stack. When a template has already been approved (the filename is not appended with `.pending`) it results to a noop. When a template is changed, the filename is changed because of the md5 hashing.
jpb
force-pushed
the
create-approve-templates
branch
from
420abd5 to
16bf034
Compare
Replace template URL when ApprovedTemplateLocation is in use
depends on #41
Usage
ApprovedTemplateLocationto the full s3 url with key of the template to be approvediidy approve-template stack-args.yamlNotes:
I could also change it to
iidy approve-template https://s3.amazon.com...yaml.pendingbut to add--profilefunctionality is more tedious.Ideally, it would be using
iidy approve-template hash.yaml.pendingbut the command would not know which profile to use and which bucket to look at.Added a flag
--approveto approve the template. Running theiidy approve-template ...command should diff the changes first. I have opted not to use areadlineto precent fat fingering answers.TODO: