Skip to content

fix(helm): ensure coder can be deployed in a non-default namespace #16579

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Feb 18, 2025
Merged

fix(helm): ensure coder can be deployed in a non-default namespace #16579

merged 1 commit into from
Feb 18, 2025

Conversation

ThomasK33
Copy link
Member

@ThomasK33 ThomasK33 commented Feb 14, 2025 •
edited
Loading

Added namespace to all resources in the helm chart and added tests to ensure that coder can be deployed in non-default namespaces, as specified via the namespace flag in the helm command.

Ways to verify this:

  • current state:

    $ helm template my-coder coder -n coder --version 2.19.0 --repo https://helm.coder.com/v2 | yq '.metadata.namespace'
null
---
null
---
null
---
null
---
null

  • fixed state when checking out this PR:

    $ helm template my-coder ./helm/coder -n coder --set coder.image.tag=latest | yq '.metadata.namespace'
coder
---
coder
---
coder
---
coder
---
coder

Change-Id: Ib66d4be9bcc4984dfe15709362e1fe0dcd3e847f
Signed-off-by: Thomas Kosiewski tk@coder.com

@ThomasK33
fix(helm): ensure coder can be deployed in a non-default namespace
b7f2d12 
Change-Id: Ib66d4be9bcc4984dfe15709362e1fe0dcd3e847f
Signed-off-by: Thomas Kosiewski <tk@coder.com>
@ThomasK33 Graphite App
Copy link
Member Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@ThomasK33 ThomasK33 marked this pull request as ready for review February 14, 2025 15:27
johnstcn
johnstcn approved these changes Feb 14, 2025
View reviewed changes
Copy link
Member

@johnstcn johnstcn left a comment

Choose a reason for hiding this comment

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

LGTM, but do we need to do this for every single test case? Would simply testing with the default values suffice?

@ThomasK33
Copy link
Member Author

LGTM, but do we need to do this for every single test case? Would simply testing with the default values suffice?

I don't think testing the default values would be enough here. For example, the ingress is not enabled by default; thus, if it were to miss the namespace, we'd not be able to catch it.
The same applies to all the other objects that get conditionally templated.

@abasu0713
Copy link

I would like to add the issue affects the following CRDs for a failed deployment using Terraform helm provider:

  • RoleBinding
  • Role
  • Deployment
  • Service
Screenshot 2025-02-14 at 1 07 48 PM Screenshot 2025-02-14 at 1 07 42 PM Screenshot 2025-02-14 at 1 07 34 PM Screenshot 2025-02-14 at 1 07 27 PM

I haven't used Ingress, since my setup is Cloudflare Tunnels -> Coder Service (ClusterIP) which avoids the need for Ingress, Load balancers or NodePorts with Egress only routes within my network.
Hope this helps.
@ThomasK33 Thanks for sending the PR. You rock! Looking forward to getting this merged.

@abasu0713
Copy link

I am sorry if I am coming across rushing, but any chance we could get this merged please! 🙏

@matifali matifali linked an issue Feb 17, 2025 that may be closed by this pull request
Terraform Helm Deployment fails #16566
Closed
@ThomasK33 ThomasK33 merged commit 420855d into main Feb 18, 2025
37 checks passed
@ThomasK33 ThomasK33 deleted the 02-14-fix_helm_ensure_coder_can_be_deployed_in_a_non-default_namespace branch February 18, 2025 11:50
@github-actions github-actions bot locked and limited conversation to collaborators Feb 18, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@johnstcn johnstcn johnstcn approved these changes

@mafredri mafredri Awaiting requested review from mafredri

Assignees

@ThomasK33 ThomasK33

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Terraform Helm Deployment fails
3 participants
@ThomasK33 @abasu0713 @johnstcn