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

Make the host of AWS Code Commit changeable #24771

Closed
wants to merge 2 commits into from

Conversation

Kang3498
Copy link

@Kang3498 Kang3498 commented May 14, 2024

Hey, I just made a Pull Request!

As-Is : The catalog:register action does not work when using AWS Code Commit.

In the link below, config.host is forced to console.aws.amazon.com, so, If I set repoContentsUrl to {region}.console.aws.amazon.com in the catalog:register action, No Integration error occurs.
https://github.com/backstage/backstage/blob/master/packages/integration/src/awsCodeCommit/config.ts#L67

Instead, if I remove the region from the url, an error occurs due to the link below.
https://github.com/backstage/backstage/blob/master/packages/backend-common/src/reading/AwsCodeCommitUrlReader.ts#L70

✔️ Checklist

  • A changeset describing the change and affected packages. (more info)
  • Added or updated documentation
  • Tests for new functionality and regression tests for bug fixes
  • Screenshots attached (for UI changes)
  • All your commits have a Signed-off-by line in the message. (more info)

@Kang3498 Kang3498 requested review from a team as code owners May 14, 2024 17:05
@Kang3498 Kang3498 requested review from freben and camilaibs May 14, 2024 17:05
@backstage-goalie
Copy link
Contributor

backstage-goalie bot commented May 14, 2024

Changed Packages

Package Name Package Path Changeset Bump Current Version
@backstage/integration packages/integration patch v1.11.0

@backstage-goalie
Copy link
Contributor

Thanks for the contribution!
All commits need to be DCO signed before they are reviewed. Please refer to the the DCO section in CONTRIBUTING.md or the DCO status for more info.

Signed-off-by: SangKyu Kang <kang3498@hanmail.net>
Signed-off-by: SangKyu Kang <kang3498@hanmail.net>
@stijnbrouwers
Copy link
Contributor

stijnbrouwers commented May 18, 2024

@Kang3498
I had a fix ready for #24824 which is a similar issue to this one. You can view my fix here:
stijnbrouwers#1
(it's a pull request to my own master branch just for you to have an easy view on my code)

I did the same as you did and allowed the user to be able to set the host manually.
I also introduced a mandatory "region" property to the awsCodeCommit config. This would allow the user to set only a region and the system would determine the correct host URL from that.

Since I require the region, I am not adding a default entry in readAwsCodeCommitIntegrationConfigs This prevents the scenario where, for example, we default to region "us-east-1" and the user would get a message like "no integration found for url...". In this scenario, they might not really knows what's going on without having to debug the code...

@Kang3498
Copy link
Author

Kang3498 commented May 20, 2024

@stijnbrouwers
That's a good suggestion. If you create a new PR with that suggestion, I will close this PR.

No matter what method we use, we need to merge quickly to use Code Commit. Currently, we are not at the level where we can say that Code Commit is supported...

@stijnbrouwers
Copy link
Contributor

@Kang3498
I just created the PR #24843

Feel free to tag me in the future when you find an issue regarding AWS CodeCommit integration. Should I have the time, I will definitely take a look.

@Kang3498 Kang3498 closed this May 21, 2024
@Kang3498
Copy link
Author

This will be replaced with the PR #24843

Copy link
Contributor

Uffizzi Cluster pr-24771 was deleted.

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.

None yet

2 participants