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.

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

feature branch deployments #112

Open
wants to merge 22 commits into
base: ci-cd
Choose a base branch
from
Open

feature branch deployments #112

wants to merge 22 commits into from

Conversation

brucehyslop
Copy link

  • setup cognito app client based for environment (externalised of app client)
  • ported configuration of stack outputs from export config code build into cloudfromation template Fn::ImportValue
  • externalisation of ui build config

- AppClientBucket:
Fn::ImportValue:
Fn::Sub: ${pCognitoStackName}-AppClientBucket
TimeoutInMinutes: 5
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems like it belongs in the deploy component rather than base as it's only relevant to the application. Then the pAppClientDomain can be got from the existing sub_domain / hosted_zone thats already available in that component

Value: !Sub
- lists-${ResourceName}
- ResourceName: !If [ IsDev, !Ref pCleanBranch, !Ref pEnvironment ]
Copy link
Contributor

Choose a reason for hiding this comment

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

I intentionally used the !Ref ListsSecret so the name wouldn't have to be constructed twice, it's a bit of a gotcha if the secrets name is changed, it had to be changed identically here as well. The downside is the secrets name has to parsed from the URI but i thought that was the lesser of the evils

@@ -15,6 +15,9 @@ SLACK_DEPLOY_NOTIFICATION = true
SLACK_ALERT_CHANNEL = zabbix-lists
AUTO_DEPLOY = true

COGNITO_STACK_NAME = ala-cognito-pool-external-app-clients
APP_CLIENT_DOMAIN = {PRODUCT_NAME}-${CLEAN_BRANCH}.${HOSTED_ZONE}
Copy link
Contributor

Choose a reason for hiding this comment

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

is this a duplicate of the app client domain used in the deploy component?

- UserPoolId:
Fn::ImportValue:
Fn::Sub: "${pBaseStackName}-UserPoolId"

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh cool, that looks like a good way to include imports as environment vars in the JSON. Is it possible to retain the function shortform for !Sub vs Fn::Sub at the beginning?

@@ -8,23 +8,33 @@ REGION = ap-southeast-2
BOOTSTRAP_STACK_NAME = ala-bedrock-cicd-bootstrap-production
BUCKETS_STACK_NAME = ala-bedrock-cicd-buckets-production
VPC_STACK_NAME = ala-bedrock-vpc-production
REGOLITH_STACK_NAME = ala-regolith-cluster-testing
REGOLITH_STACK_NAME = ala-regolith-cluster-testing-testing
Copy link
Contributor

Choose a reason for hiding this comment

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

there's a PR in to fix the duplication in the REGOLITH_STACK_NAME

@joe-lipson
Copy link
Contributor

Looks good Bruce. I still wonder about using the CLI to create the app clients rather than CloudFormation, it would remove the dependency of having the nested template managed in a different repo and having to keep it up to date there

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants