Ability to use codestar sources in deployment map #312
Conversation
| properties: | ||
| repository: my-ecs-app # Optional, above name property will be used if this is not specified | ||
| owner: github-enterprise-team-org | ||
| codestar_connection_path: /ghes/connection1/arn # The path in AWS Systems Manager Parameter Store that holds the AWS CodeStar Connection arn |
There was a problem hiding this comment.
This looks like a path used in an organization. Can you make it easier to read so people will understand they can change the value to their own preference? In its current form it looks like I cannot change it, while you can, am I right?
There was a problem hiding this comment.
yes, this is path is arbitrary. will update it to /path/to/parameter
| owner: github-enterprise-team-org | ||
| codestar_connection_path: /ghes/connection1/arn # The path in AWS Systems Manager Parameter Store that holds the AWS CodeStar Connection arn | ||
| params: | ||
| notification_endpoint: [email protected] |
There was a problem hiding this comment.
To prevent spam and mixing different email examples in our code base, can you change this to [email protected]?
| params: | ||
| notification_endpoint: [email protected] | ||
| targets: | ||
| - [ /ecoin/testing, /ephone/testing ] |
There was a problem hiding this comment.
Same here, can you change this to be more generic?
| try: | ||
| response = ssm_client.get_parameter(Name=codestar_connection_path) | ||
| except Exception as e: | ||
| print(f"No parameter found at {codestar_connection_path}. Check the path/value.") |
There was a problem hiding this comment.
Could you change this print to use the logger instead. Like we do here? But using error instead.
| @@ -436,6 +436,10 @@ Resources: | |||
| Resource: | |||
| - !Sub arn:aws:codepipeline:${AWS::Region}:${AWS::AccountId}:webhook:adf-webhook-* | |||
| - !Sub arn:aws:codepipeline:${AWS::Region}:${AWS::AccountId}:${PipelinePrefix}* | |||
| - Effect: Allow | |||
| Action: | |||
| - "codestar-connections:*" | |||
There was a problem hiding this comment.
Can you change this to list the actions that it really needs, instead of all?
| - Effect: Allow | ||
| Sid: "AllowCodeStarConnections" | ||
| Action: | ||
| - "codestar-connections:*" |
There was a problem hiding this comment.
Same here, please list the exact actions this policy will need.
| The AWS CodeStar connection needs to already exist and be in the "Available" Status. To use the AWS CodeStar Connection with ADF, its arn needs to be stored in AWS Systems Manager Parameter Store in the deployment account's main region (see details below). Read the CodePipeline documentation for more [information on how to setup the connection](https://docs.aws.amazon.com/dtconsole/latest/userguide/getting-started-connections.html). | ||
|
|
||
| Provider type: `codestar`. | ||
| #### Properties |
There was a problem hiding this comment.
Can you add a newline above the Properties title here?
Issue #, if available:
Description of changes:
Added the ability to use AWS Codestar as the source for pipelines defined in the deployment map.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.