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

fix(ecs-patterns): integ test failed with certificate error #29649

Closed
wants to merge 2 commits into from

Conversation

wafuwafu13
Copy link
Contributor

@wafuwafu13 wafuwafu13 commented Mar 29, 2024

Issue # (if applicable)

Part of: #29186 (comment)

Reason for this change

aws-ecs-patterns/test/fargate/integ.alb-fargate-service-https was failed with certificate error (#29186 (comment)) because domainName, hostedZoneId, zoneName are hard-coded.

Description of changes

  • Set domainName, hostedZoneId, zoneName to environment variable
    ref:

    const hostedZoneId = process.env.CDK_INTEG_HOSTED_ZONE_ID ?? process.env.HOSTED_ZONE_ID;
    if (!hostedZoneId) throw new Error('For this test you must provide your own HostedZoneId as an env var "HOSTED_ZONE_ID". See framework-integ/README.md for details.');
    const hostedZoneName = process.env.CDK_INTEG_HOSTED_ZONE_NAME ?? process.env.HOSTED_ZONE_NAME;
    if (!hostedZoneName) throw new Error('For this test you must provide your own HostedZoneName as an env var "HOSTED_ZONE_NAME". See framework-integ/README.md for details.');
    const domainName = process.env.CDK_INTEG_DOMAIN_NAME ?? process.env.DOMAIN_NAME;
    if (!domainName) throw new Error('For this test you must provide your own DomainName as an env var "DOMAIN_NAME". See framework-integ/README.md for details.');

  • enableECSManagedTags and enableECSManagedTags properties are not needed for test https

  • Run yarn integ

Description of how you validated changes

  • Pass integ test

Checklist


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 valued-contributor [Pilot] contributed between 6-12 PRs to the CDK p2 labels Mar 29, 2024
@aws-cdk-automation aws-cdk-automation requested a review from a team March 29, 2024 09:57
@wafuwafu13
Copy link
Contributor Author

Build is failed, but I think destructive changes are necessary.

@aws-cdk-testing/framework-integ: Failed: /codebuild/output/src3468774902/src/github.com/aws/aws-cdk/packages/@aws-cdk-testing/framework-integ/test/aws-ecs-patterns/test/fargate/integ.alb-fargate-service-https.js
@aws-cdk-testing/framework-integ: !!! This test contains destructive changes !!!
@aws-cdk-testing/framework-integ:     Stack: aws-ecs-integ-alb-fg-https - Resource: myServiceCertificate152F9DDA - Impact: WILL_REPLACE
@aws-cdk-testing/framework-integ:     Stack: aws-ecs-integ-alb-fg-https - Resource: myServiceDNSD76FB53A - Impact: WILL_REPLACE
@aws-cdk-testing/framework-integ: !!! If these destructive changes are necessary, please indicate this on the PR !!!
@aws-cdk-testing/framework-integ: Error: Some changes were destructive!
@aws-cdk-testing/framework-integ:     at main (/codebuild/output/src3468774902/src/github.com/aws/aws-cdk/packages/@aws-cdk/integ-runner/lib/cli.js:183:15)
@aws-cdk-testing/framework-integ: Error: integ-runner exited with error code 1
@aws-cdk-testing/framework-integ: Tests failed. Total time (1m52.8s) | integ-runner (1m39.2s) | /codebuild/output/src3468774902/src/github.com/aws/aws-cdk/node_modules/jest/bin/jest.js (12.4s)

@TheRealAmazonKendra
Copy link
Contributor

Build is failed, but I think destructive changes are necessary.

@aws-cdk-testing/framework-integ: Failed: /codebuild/output/src3468774902/src/github.com/aws/aws-cdk/packages/@aws-cdk-testing/framework-integ/test/aws-ecs-patterns/test/fargate/integ.alb-fargate-service-https.js
@aws-cdk-testing/framework-integ: !!! This test contains destructive changes !!!
@aws-cdk-testing/framework-integ:     Stack: aws-ecs-integ-alb-fg-https - Resource: myServiceCertificate152F9DDA - Impact: WILL_REPLACE
@aws-cdk-testing/framework-integ:     Stack: aws-ecs-integ-alb-fg-https - Resource: myServiceDNSD76FB53A - Impact: WILL_REPLACE
@aws-cdk-testing/framework-integ: !!! If these destructive changes are necessary, please indicate this on the PR !!!
@aws-cdk-testing/framework-integ: Error: Some changes were destructive!
@aws-cdk-testing/framework-integ:     at main (/codebuild/output/src3468774902/src/github.com/aws/aws-cdk/packages/@aws-cdk/integ-runner/lib/cli.js:183:15)
@aws-cdk-testing/framework-integ: Error: integ-runner exited with error code 1
@aws-cdk-testing/framework-integ: Tests failed. Total time (1m52.8s) | integ-runner (1m39.2s) | /codebuild/output/src3468774902/src/github.com/aws/aws-cdk/node_modules/jest/bin/jest.js (12.4s)

Oh, this is totally fine because you are changing the integ test and not the resource. You can run them again with --disable-update-workflow added in your command and the destructive changes warning should go away.

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
  • Commit ID: 5f74954
  • Result: FAILED
  • Build Logs (available for 30 days)

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

@wafuwafu13 wafuwafu13 marked this pull request as draft April 9, 2024 20:57
mergify bot pushed a commit that referenced this pull request Apr 17, 2024
### Issue # (if applicable)

Part of: #29186 (comment)

### Reason for this change


`aws-ecs-patterns/test/fargate/integ.alb-fargate-service-https-idle-timeout` was failed with certificate error (#29186 (comment)).
It is enough to set `idleTimeout` property to test loadbalancer with idleTimeout.

`aws-ecs-patterns/test/fargate/integ.alb-fargate-service-https` is also failed with certificate error so I will fix at another PR #29649.

### Description of changes


- Rename `aws-ecs-patterns/test/fargate/integ.alb-fargate-service-https-idle-timeout.ts` to `aws-ecs-patterns/test/fargate/integ.alb-fargate-service-idle-timeout.ts`
- Remove unnecessary property
- Run `yarn integ`

### Description of how you validated changes


- Pass integ test

### Checklist
- [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@wafuwafu13 wafuwafu13 closed this Apr 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p2 valued-contributor [Pilot] contributed between 6-12 PRs to the CDK
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants