-
Notifications
You must be signed in to change notification settings - Fork 490
Add configuration for staging repo #2141
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
Conversation
update bootstrap
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.
Pull Request Overview
This pull request adds support for staging repository configuration to the AWS Lambda container images infrastructure pipeline. The changes enable the system to deploy from both main and staging GitHub repositories by introducing new parameters and duplicating pipeline creation logic.
Key Changes
- Added staging repository parameters (owner, name, branch) to bootstrap script and infrastructure
- Modified pipeline creation to support multiple repositories with parameterized GitHub source configurations
- Refactored Configuration.ProjectName from static to instance property
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
bootstrap.ps1 | Adds staging repository parameters and environment variable assignments |
SelfMutatingPipelineStack.cs | Adds staging repository environment variables to build configuration and fixes Configuration.ProjectName reference |
Program.cs | Updates Configuration.ProjectName reference to use instance property |
PipelinesStage.cs | Refactors pipeline creation to support both main and staging repositories |
PipelineStack.cs | Parameterizes GitHub source configuration and updates pipeline naming |
Configuration.cs | Adds staging repository properties and converts ProjectName to instance property |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
}; | ||
|
||
// Self mutation | ||
|
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.
[nitpick] Remove this empty line as it adds unnecessary whitespace without improving readability.
Copilot uses AI. Check for mistakes.
public Ecrs Ecrs { get; } = new Ecrs(); | ||
public const string ProjectRoot = "LambdaRuntimeDockerfiles/Infrastructure/src/Infrastructure"; | ||
public static readonly string ProjectName = "aws-lambda-container-images"; | ||
public string ProjectName { get; } = "aws-lambda-container-images"; |
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.
made non static to be consistent
|
||
new PipelineStack(this, | ||
configuration.Frameworks[i].Framework, | ||
$"{pipelinePrefix}-{configuration.Frameworks[i].Framework}", |
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.
cloudformation seems to automatically uses the parent pipeline name+ this for the name of the stack and we get something like
aws-lambda-container-images-aws-lambda-container-images-staging-net9
but i dont really care about the stack name too much to be honest. This was the only way i found to make the pipeline name + what appears in the ui for the pipeline in the main pipeline look correct
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.
im not a fan of this, but not a big deal
public string GitHubRepository { get; } = Environment.GetEnvironmentVariable("AWS_LAMBDA_GITHUB_REPO_NAME"); | ||
public string GitHubBranch { get; } = Environment.GetEnvironmentVariable("AWS_LAMBDA_GITHUB_REPO_BRANCH"); | ||
|
||
public string GitHubOwnerStaging { get; } = Environment.GetEnvironmentVariable("AWS_LAMBDA_GITHUB_REPO_OWNER_STAGING"); |
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 didnt bother to move all of these to a json file yet because we would still need to send in the token key as env variables anyway. i may come back to this in the future
|
||
new PipelineStack(this, | ||
configuration.Frameworks[i].Framework, | ||
$"{pipelinePrefix}-{configuration.Frameworks[i].Framework}", |
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.
im not a fan of this, but not a big deal
Add configuration for staging repo. Please note that this change does not include changes to auto fetch the PAT when it rotates. That will be in a follow up PR. i wasnt able to test end to end fully in my personal account (building the images and publishing to ecr) because my account didnt have permissions. but this change only modifies the pipeline creation so it should work.
In the below picture we can see the staging pipelines get created (the one is in failed state because the webhook limit of 20 was reached but that is fine for testing)
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.