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

feature: added namespace helper #1849

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Bugs5382
Copy link

So I am working on a project that puts Harbor inside the same namespace as the project, instead of being in default. I was looking trhough the helm chart and noticed that there was no way to override the namespace. Thus, I wrote this up. I tested it locally, and everything seems to be working A-OK!

Thanks!!!

@Vad1mo
Copy link
Member

Vad1mo commented Oct 27, 2024

@Bugs5382 the your PR does not have a valid DCO

@Bugs5382
Copy link
Author

@Vad1mo Completed. Sorry about that.

templates/_helpers.tpl Show resolved Hide resolved
@MinerYang MinerYang self-assigned this Oct 28, 2024
@MinerYang
Copy link
Collaborator

Hi @Bugs5382 ,

Would explicitly specify the namespace while you installing satisfy your requirements? like...

helm install harbor ... -n test

Copy link
Contributor

@reasonerjt reasonerjt left a comment

Choose a reason for hiding this comment

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

Per latest comment by @MinerYang
This PR probably should not be merged into upstream.
I'm blocking this PR until we have better clarity.

@Bugs5382
Copy link
Author

Bugs5382 commented Nov 9, 2024

Per latest comment by @MinerYang This PR probably should not be merged into upstream. I'm blocking this PR until we have better clarity.

So the reason why I request this change as if your doing a helm template output, even specifying the namespace, the default one gets applied. Which is kinda hard when I am using this chart with krustomize and everything else. I updated the chart.

Copy link
Author

@Bugs5382 Bugs5382 left a comment

Choose a reason for hiding this comment

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

Compelted.

@Bugs5382 Bugs5382 requested a review from reasonerjt November 9, 2024 00:22
- allows users to override the namespace

Signed-off-by: Bugs5382 <[email protected]>
- simplified

Signed-off-by: Bugs5382 <[email protected]>
@MinerYang
Copy link
Collaborator

MinerYang commented Nov 11, 2024

Hi @Bugs5382 ,
I understood this is a limitation from helm. Thanks for your contribution!

@MinerYang
Copy link
Collaborator

Hi @Bugs5382 ,

Another thing may need your help and we would appreciate if you could add some UT test cases including this changes here:
https://github.com/goharbor/harbor-helm/tree/main/test/unittest.

@Bugs5382
Copy link
Author

@MinerYang I'll check it out today!

@Kajot-dev
Copy link
Contributor

Kajot-dev commented Nov 25, 2024

What is the purpose of overriding a namespace? My question is: Is there a situation where you want to have a different release namespace than an actual install namespace?

@Vad1mo
Copy link
Member

Vad1mo commented Dec 7, 2024

So the reason why I request this change as if your doing a helm template output, even specifying the namespace, the default one gets applied. Which is kinda hard when I am using this chart with krustomize and everything else. I updated the chart.

we are too running helm template . and I don't see that behavior, instead I see correct namespaces.

@Kajot-dev
Copy link
Contributor

So the reason why I request this change as if your doing a helm template output, even specifying the namespace, the default one gets applied. Which is kinda hard when I am using this chart with krustomize and everything else. I updated the chart.

we are too running helm template . and I don't see that behavior, instead I see correct namespaces.

Running helm template . with the -n/--namespace option works for me too

@vpechor
Copy link

vpechor commented Jan 8, 2025

What is the purpose of overriding a namespace? My question is: Is there a situation where you want to have a different release namespace than an actual install namespace?

In our environment, we use strict GitOps approach using ArgoCD. Most of installed applications are templated using helm template command which does not offer the possibility to use .Release variables as helm install does. Using -n/--namespace is an option for small projects, but is not viable for bigger helm charts, where each module gets its own namespace (e.g. when using umbrella helm chart).
Additionally using ArgoCD to use helm install is an option, but not preferred for bigger projects as there's no direct control of what gets to production and has introduced us to some nasty bugs in production in the past (which is of course user issue).

namespaceOverride would be greatly appreciated.

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.

6 participants