fix(cdk-assets): remove https from endpoint, to work with podman

[janmeier]

This PR is a redo of #19115

podman does not allow prefixing the endpoint with https when calling docker login:

fail: docker login --username AWS --password-stdin https://XXX.dkr.ecr.eu-west-1.amazonaws.com exited with error code 125: Error: credentials key has https[s]:// prefix

This PR removes the prefix, which works with both docker and podman

Closes #16209

---

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

As an aside, I was not able to run the unit tests locally, the would fail with Cannot read asset manifest at '/simple/cdk.out': ENOENT: no such file or directory, stat '/simple/cdk.out', seeming to suggest that mock-fs is not working as expected. However, I removed the https prefix from all existing tests, and will cross my fingers and see what the CI says.

[aws-cdk-automation]

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

[janmeier]

Exemption Request: I do not think this change qualifies for an integration test. It should not change the output of cdk synth.

Additionally, the PR already changes unit tests, which should be enough to catch future regressions

[janmeier]

Turns out mock-fs does not work with node 20 tschaub/mock-fs#384

Managed to fix the latest failing test 🤞

[aws-cdk-automation]

This PR has been in the CHANGES REQUESTED state for 3 weeks, and looks abandoned. To keep this PR from being closed, please continue work on it. If not, it will automatically be closed in a week.

[janmeier]

@aws-cdk-automation Can I talk to a human please? I believe my PR is ready for review

[aws-cdk-automation]

This PR has been deemed to be abandoned, and will be automatically closed. Please create a new PR for these changes if you think this decision has been made in error.

[TheRealAmazonKendra]

@aws-cdk-automation Can I talk to a human please? I believe my PR is ready for review

Hello, you've reached a CDK Human. We cannot come to the PR right now so please leave a... just kidding.

Apologies for the delay on this review!

[TheRealAmazonKendra]

I don't think that we can just remove the https across the board as it might cause issues elsewhere. We do have an existing integ test for deploying with a docker asset so I'm going to start with running this through our test pipeline.

[TheRealAmazonKendra]

@Mergifyio update

[mergify]

update

❌ Mergify doesn't have permission to update

For security reasons, Mergify can't update this pull request. Try updating locally.
GitHub response: refusing to allow a GitHub App to create or update workflow .github/workflows/request-cli-integ-test.yml without workflows permission

[janmeier]

@aws-cdk-automation Can I talk to a human please? I believe my PR is ready for review

Hello, you've reached a CDK Human. We cannot come to the PR right now so please leave a... just kidding.

Apologies for the delay on this review!

No worries, I've been able to patch my local installation in the meantime, but looking forward to getting this merged 🕺

[janmeier]

I don't think that we can just remove the https across the board as it might cause issues elsewhere. We do have an existing integ test for deploying with a docker asset so I'm going to start with running this through our test pipeline.

According to a comment in the origin issue, docker should be able to run without https #16209 (comment). I'm hoping the integration tests agree 🤞

[araker]

Would be nice if this can be merged. I currently cannot build assets within CDK.

[janmeier]

@TheRealAmazonKendra Friendly ping - Is there anything I can do to bring this over the finish line?

[TheRealAmazonKendra]

@Mergifyio update

[mergify]

update

❌ Mergify doesn't have permission to update

For security reasons, Mergify can't update this pull request. Try updating locally.
GitHub response: refusing to allow a GitHub App to create or update workflow .github/workflows/github-merit-badger.yml without workflows permission

[TheRealAmazonKendra]

Sorry for the radio silence on this one. We are moving cdk-assets out of the main repo and into a repo of it's own, which is still in progress. As long as the test run I'm doing now works, I'll happily move this over as well but I wanted to get your preference on how we go about that. I can move it for you so you don't have to go to the extra effort, but that removes the credit to you in GitHub for the change. Alternatively, you can open a new PR in the new repo, but that's obviously extra work on your end.

Any preference here?

[aws-cdk-automation]

➡️ PR build request submitted to test-main-pipeline ⬅️

A maintainer must now check the pipeline and add the pr-linter/cli-integ-tested label once the pipeline succeeds.

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

[aws-cdk-automation]

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
• Commit ID: e958e5e
• Result: SUCCEEDED
• Build Logs (available for 30 days)

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

[janmeier]

Sorry for the radio silence on this one. We are moving cdk-assets out of the main repo and into a repo of it's own, which is still in progress. As long as the test run I'm doing now works, I'll happily move this over as well but I wanted to get your preference on how we go about that. I can move it for you so you don't have to go to the extra effort, but that removes the credit to you in GitHub for the change. Alternatively, you can open a new PR in the new repo, but that's obviously extra work on your end.

Any preference here?

Point me to the new repo, and I'll make a PR :)

[kaizencc]

Hi @janmeier, apologies for the mega late response. our new location for cdk-assets is here: https://github.com/aws/aws-cdk-cli/tree/main/packages/cdk-assets. it's currently private but i am assuming the link will start working when we make the repo public later this week.

i'm going to close this PR here since you've indicated you'd want to make a new PR in the new repository. if you're still willing, please feel free to make that contribution when the link becomes live!

[github-actions]

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.