Sort environment variables based on kubernetes style referencing

[esselius]

Fixes #3108

/cc @zendesk/samson

Tasks

• 👍 from team

Risks

• Level: Low

[esselius]

@grosser Does this seem appropriate? :)

[grosser]

Suggested change

	In kubernetes, you can reference an environment variable from another using the syntax: `FOO: $(BAR)`. However, in this case `BAR` needs to be defined before `FOO` for it to work. This is due to an implementation detail in kubernetes itself.
	In kubernetes, you can reference an environment variable from another using the syntax: `FOO: ${BAR}`. However, in this case `BAR` needs to be defined before `FOO` for it to work. This is due to an implementation detail in kubernetes itself.

[grosser]

... maybe I understood your proposal wrong ...
samson does env variable replacement too with ${FOO} which works for non-kubernetes projects too

[esselius]

Basically, what we want is to be able to reference $DEPLOY_GROUP, which is not available when the env-plugin runs it's resolve_dollar_variables()-method in EnvironmentVariable, triggered by the Samson::Hook in the kubernetes-plugin.

We decided to make this an optional feature to please kubernetes, enabled by setting a environment variable, rather than refactoring how the kubernetes and env (and possibly other) plugins pass around env vars through hooks without knowing if that was a appropriate design decision.

What do you think @grosser ?

[grosser]

I think the problem is that resolve_dollar_variables (and preview / resolve-secrets) belongs into base and not into the env plugins since we should only resolve once we have collected all env vars from base+plugins.

A solution would be to add a new Samson::EnvResolver class that collects env (can be given a base env too) and handles preview/merge/resolve/secrets ... then use that from template_filler and from plugins/env/app/controllers/env/environment_controller.rb.

... with that any other plugin can add env vars via deploy_group_env hook and they automatically get resolved/previewed

... sounds good ?

(base make a fresh PR for that so this here does not get too cluttered)

[esselius]

Sounds great, we'll try to produce a PR in the coming days

Should we still include the $(FOO)-based sorting logic?

[grosser]

I'd do the refactor first and then the sorting after so it's cleaner :)

…

On Tue, Jan 15, 2019 at 12:28 AM Peter Esselius ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In plugins/kubernetes/README.md <#3116 (comment)>: > @@ -222,3 +222,11 @@ this is useful when dealing with large deployments that have some random failure To debug services or to create resources that needs to reference a selector that doesn't include team/role (like a Gateway), you can disable selector validation with: `metadata.annotations.samson/service_selector_across_roles: "true"` + +### Ordering environment variables based on references + +In kubernetes, you can reference an environment variable from another using the syntax: `FOO: $(BAR)`. However, in this case `BAR` needs to be defined before `FOO` for it to work. This is due to an implementation detail in kubernetes itself. Sounds great, we'll try to produce a PR in the coming days Should we still include the $(FOO)-based sorting logic? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#3116 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAsZ4XwLtZNHhBWL2Or-mGjUFUODmsmks5vDZE1gaJpZM4Z5EBA> .