-
Notifications
You must be signed in to change notification settings - Fork 49
fix: repo info for Gitlab does not support group nesting #140
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
base: main
Are you sure you want to change the base?
Conversation
4de9f35
to
2f48d4e
Compare
2f48d4e
to
0fa98a1
Compare
@faymard - thanks for this contribution. We appreciate it. We will review it and test it before our next release. |
awslabs/harmonix#140 Co-authored-by: Florian JUDITH <[email protected]>
* Fixed repo info for Gitlab does not support group nesting awslabs/harmonix#140 * Bump Backstage framework version 1.35.1 * Updated api reports * Fixed alpha endpoints * Fixed lockfile duplicates --------- Co-authored-by: Florian JUDITH <[email protected]>
I am bumping into the exact same issue. |
We do not have a hard date at this time for the next Harmonix release. We have internal discussions going on with regard to this. In the mean time, I suggest going forward with your own repo fork. Thanks a lot for the feedback and contributions. They mean a lot to our team! |
Hi @acwatson, thank you for answering.
May I ask what these "internal discussions" are about ? Are they about the future of the product itself ? Or rather about the way of contributing on it ? |
.gitignore
Outdated
@@ -141,3 +141,6 @@ lerna-debug.log* | |||
!environment/.environment-shared.json | |||
|
|||
/backstage | |||
|
|||
backstage-plugins/**/*.d.ts |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have a file that this would prevent from being committed: backstage-plugins/plugins/aws-apps-common/config.d.ts
This file is needed for configuring the plugin as per https://backstage.io/docs/conf/defining/
I looked to see what other plugins are doing and I also see the config.d.ts file committed there as well - example - https://github.com/backstage/backstage/blob/master/plugins/catalog-backend-module-github/config.d.ts
I don't think this gitignore rule should be there. Am I wrong on this? Can you clarify?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, yeah that must be a mistake. That must have came from when I tried some things with tsc
without having set the output directory and did not want to commit the results.
Removed 👍
@faymard - can you provide a little more context here? The way Harmonix is coded for GitLab, we created 4 different groups: aws-app, aws-environments, aws-environment-providers, and aws-resources. We made these groups so that it would be easier to find related things and also so that permissions could be applied based on the group. For example, only "platform engineers" can modify repositories under aws-environment-providers. Can you give an example of a group and repo name that you want to use that wasn't working? |
Hi, so the context is that we already had Backstage and GitLab set up independently, we wanted to integrate Harmonix as smoothly as possible. Using four root groups as you listed was not possible, we had to be nested in a subgroup. Permissions work the same way, it's just that the repositories are located in something like The underlying way that Harmonix is intended to work and that you described remains unchanged. Does that clarify enough ? |
That makes sense and helps me think about how to test your changes. I can't make a decision just by looking at the code, I need to try it out to better assess. It looks promising though and we definitely want Harmonix to be able to work with existing GitLab instances and allow users to set up GitLab in a way that works for them. |
@@ -55,8 +55,12 @@ | |||
"@backstage/core-plugin-api": "^1.9.3", | |||
"@backstage/errors": "^1.2.4", | |||
"@backstage/plugin-catalog": "^1.21.1", | |||
"@backstage/plugin-catalog-graph": "^0.4.14", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these changes needed for GitLab group nesting? My guess is that they are not. Could you please do a review of this PR and try to remove any changes that are not needed for GitLab group nesting?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, done.
What does this PR do?
🛑 Please open an issue first to discuss any significant work and flesh out details/direction - we would hate for your time to be wasted.
Consult the CONTRIBUTING guide for submitting pull-requests.
git-util.ts
to support hosting Harmonix reference repositories in a nested Gitlab groupGitlabApi::getFileContents
(parse base 64) andGitlabApi::commitContent
(redundant json() call)Motivation
We are working on integrating Harmonix in an existing Backstage and GitLab environment. Everything went surprisingly fine with installing/integrating the plugins and hosting Harmonix templates, but we are encountering an issue when deleting resources from Backstage.
It seems that it comes from the way repository name and group are computed. This PR aims to correct it to support different nesting levels in GitLab.
GitHub is not impacted because deep nesting is not possible (it is always organization/repo) but I thought it would be better to unify how these informations are queried.
For Moderators
Additional Notes