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

feat(amplify-domain): Added config for auto subdomain creation #13342

Merged
merged 10 commits into from
Mar 11, 2021

Conversation

janario
Copy link
Contributor

@janario janario commented Mar 2, 2021


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

@github-actions github-actions bot added the @aws-cdk/aws-config Related to AWS Config label Mar 2, 2021
@mergify
Copy link
Contributor

mergify bot commented Mar 2, 2021

Title does not follow the guidelines of Conventional Commits. Please adjust title before merge.

Copy link
Contributor

@MrArnoldPalmer MrArnoldPalmer left a comment

Choose a reason for hiding this comment

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

Some small changes. Additionally one of the tests fails to compile because, but I believe this will be fixed by making enabledAutoSubdomain optional.

packages/@aws-cdk/aws-amplify/lib/domain.ts Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-amplify/lib/domain.ts Outdated Show resolved Hide resolved
@gitpod-io
Copy link

gitpod-io bot commented Mar 4, 2021

@mergify mergify bot dismissed MrArnoldPalmer’s stale review March 4, 2021 07:48

Pull request has been modified.

@janario janario changed the title Added config for auto subdomain creation feat amplify-domain: Added config for auto subdomain creation Mar 4, 2021
@janario janario force-pushed the feature/enable-auto-subdomain-config branch from 9a4b40d to 4959332 Compare March 4, 2021 07:52
@janario janario force-pushed the feature/enable-auto-subdomain-config branch from 4959332 to 19ac74d Compare March 4, 2021 07:53
@janario janario changed the title feat amplify-domain: Added config for auto subdomain creation feat(amplify-domain): Added config for auto subdomain creation Mar 4, 2021
@janario
Copy link
Contributor Author

janario commented Mar 6, 2021

@MrArnoldPalmer Review suggestion done, as well as lint, test, doc and build fixes.

Please, give it another review ;-)

@@ -106,6 +127,9 @@ export class Domain extends Resource {
appId: props.app.appId,
domainName,
subDomainSettings: Lazy.any({ produce: () => this.renderSubDomainSettings() }, { omitEmptyArray: true }),
enableAutoSubDomain: !!props.enableAutoSubdomain,
autoSubDomainCreationPatterns: props.autoSubdomainCreationPatterns || ['*', 'pr*'],
Copy link
Contributor

Choose a reason for hiding this comment

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

This makes it so that only when a user passes enableAutoSubdomain: true all branches will automatically get their own subdomains deployed right? if enableAutoSubDomain: false this default doesn't really matter right?

This is fine with me I'm just making sure.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From the web interface that's the behavior, by enabling enableAutoSubDomain=true all branches listen. But when I tried from the CloudFormation without providing autoSubdomainCreationPatterns it didn't seem to get a default so I tried to keep a similar behaviour

and yes, when enableAutoSubDomain=false the autoSubdomainCreationPatterns is filled but ignored

@mergify
Copy link
Contributor

mergify bot commented Mar 11, 2021

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject89A8053A-LhjRyN9kxr8o
  • Commit ID: 13b9884
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@mergify
Copy link
Contributor

mergify bot commented Mar 11, 2021

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

This was referenced Mar 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-config Related to AWS Config
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants