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

Deprecate Namespace Resource in Helm Chart in favour of helms --creat… #214

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

christophebeling
Copy link

Creating namespaces within Helm charts can introduce significant issues related to security, deployment consistency, and best practices.

Assumptions About the Default Namespace

When a Helm chart dynamically creates a namespace, it often assumes that the chart itself is running in the default namespace. This is problematic because best practices dictate avoiding the default namespace for deployments to maintain a clean and organized cluster. This assumption can lead to resources being created from within the default namespace, which is not a secure or scalable practice.

Complications with Namespace Management

If the chart is not run from the default namespace, this approach forces the creation of a new namespace from an existing one, potentially leading to an unnecessary proliferation of namespaces. For instance, if you deploy the Helm chart from a dedicated namespace, it would create yet another namespace, resulting in a cluttered environment with multiple namespaces—one for the Helm chart and another for the deployed resources.

Security Concerns

Allowing Helm charts to create namespaces can bypass security controls, especially in environments where developers and operators lack permissions to create namespaces. This can lead to unauthorized changes and the creation of unintended or insecure environments.

Deployment to Specific Namespaces

Deploying to specific, predetermined namespaces ensures resources are placed in the correct environment, such as staging or production. Dynamically creating namespaces risks deploying resources into unexpected namespaces, leading to misconfigurations and operational overhead.

Avoiding the Default Namespace

Using dynamic namespaces can result in deployments ending up in the default namespace, which is discouraged. The default namespace can become cluttered, pose security risks, and complicate resource management, particularly in environments with multiple applications or teams.

Best Practices

Best practices suggest managing namespaces separately from Helm charts, specifying the target namespace during deployment. This approach ensures resources are deployed consistently and securely within existing, approved namespaces, avoiding the pitfalls of dynamic namespace creation.

@christophebeling christophebeling requested a review from a team as a code owner August 14, 2024 20:16
@andersonshatch
Copy link
Contributor

Hi @christophebeling thanks for this contribution! It's looking good in my testing.
We'd probably want to coordinate docs and TeamServer updates to reference the Helm namespace/create-namespace flags, before we mark it as deprecated.

There's a few mentions of .Values.namespace in the NOTES.txt that'll need to be updated still.

At the moment the chart defaults deploy AgentInjectors to one namespace, [default]. I wonder if this should be changed to default to the helm namespace as well... that'd probably make the most sense, so you're good to go labelling your deployments in the same namespace you put the operator. Thoughts?

@jwelsch
Copy link

jwelsch commented Aug 23, 2024

We are prioritizing a review of this PR for our next sprint, which runs 8/26 - 9/6.

@christophebeling Would you be so kind as to answer @andersonshatch 's question above so we can move ahead on this PR, please? Thanks!

@christophebeling
Copy link
Author

Hi @christophebeling thanks for this contribution! It's looking good in my testing. We'd probably want to coordinate docs and TeamServer updates to reference the Helm namespace/create-namespace flags, before we mark it as deprecated.

There's a few mentions of .Values.namespace in the NOTES.txt that'll need to be updated still.

At the moment the chart defaults deploy AgentInjectors to one namespace, [default]. I wonder if this should be changed to default to the helm namespace as well... that'd probably make the most sense, so you're good to go labelling your deployments in the same namespace you put the operator. Thoughts?

IMO the injector should not be deployed in the same namespace as the injector, since another helm chart might deploy the namespace of the application you want to test. Many modern applications, especially microservices, utilise multiple namespaces as well.

@andersonshatch
Copy link
Contributor

I think that's a different concern. Right now the operator has to have AgentInjector resources in the same namespace as the deployment/statefulset etc that you want it to inject agents into. There's some open feature requests to have this altered so its easier to work with different namespaces.

My question was on the namespace the helm chart deploys AgentInjectors to - it's currently set to deploy them to the default namespace here - my hunch is this should be changed to default to the release namespace instead, since the objective of this PR is to stop polluting the default namespace.

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.

3 participants