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

feat: Add MYKS_HELM_NAMESPACE to environment passed to plugins #286

Closed
wants to merge 2 commits into from

Conversation

fritzduchardt
Copy link
Collaborator

I am using myks for my home automation. I have Argo disabled and roll out with kapp via myks plugin. However, kapp errors on non-existent namespaces. Therefore, I need a means to create the beforehand.

@fritzduchardt fritzduchardt changed the title Add MYKS_HELM_NAMESPACE to environment passed to plugins feat: Add MYKS_HELM_NAMESPACE to environment passed to plugins May 11, 2024
@fritzduchardt fritzduchardt requested review from Zebradil and kbudde and removed request for Zebradil May 11, 2024 14:12
@kbudde
Copy link
Member

kbudde commented May 11, 2024

Code is ok, I'm just not sure if this is the way it should be done.

  1. It's running ytt to generate helm values outside of the render stage
  2. It's only for helm and not for all sources

I'm doing something similar during rendering with ytt.
Isn't this good enough?

PS: I was thinking about providing a common set of features/functions in ytt. Maybe this could be something.

@Zebradil
Copy link
Member

I also just create namespaces with ytt when needed, no matter whether ArgoCD is enabled or not.

What if we don't just expose one specific configuration value, but provide a way to inspect the full configuration? Something like MYKS_APP_CONFIG=.myks/envs/<path-to-the-app>/config.yaml (which we'll have to write on disk beforehand). This way we'll automatically provide access to all possible fields in the config. A user then can use yq to extract required values.

@fritzduchardt
Copy link
Collaborator Author

Code is ok, I'm just not sure if this is the way it should be done.

  1. It's running ytt to generate helm values outside of the render stage
  2. It's only for helm and not for all sources

I'm doing something similar during rendering with ytt. Isn't this good enough?

PS: I was thinking about providing a common set of features/functions in ytt. Maybe this could be something.

Code is ok, I'm just not sure if this is the way it should be done.

  1. It's running ytt to generate helm values outside of the render stage
  2. It's only for helm and not for all sources

I'm doing something similar during rendering with ytt. Isn't this good enough?

PS: I was thinking about providing a common set of features/functions in ytt. Maybe this could be something.

Nice solution with ytt. I will go with that. So, there is no need for this MR.

In defense of this MR (purely academic): ytt is run several times during initialization already, it is not invoked only during rendering. None of the other sources have a namespace value in the global values schema, so there is nothing we could write into the environment.

@fritzduchardt
Copy link
Collaborator Author

I also just create namespaces with ytt when needed, no matter whether ArgoCD is enabled or not.

What if we don't just expose one specific configuration value, but provide a way to inspect the full configuration? Something like MYKS_APP_CONFIG=.myks/envs/<path-to-the-app>/config.yaml (which we'll have to write on disk beforehand). This way we'll automatically provide access to all possible fields in the config. A user then can use yq to extract required values.

I like the idea of exposing the entire configuration, but doing so in the environment would be much more user friendly. Let's think about is again, once another use case for this emerges.

@Zebradil Zebradil deleted the helm-namespace-for-plugins branch May 18, 2024 10:31
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