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

Support unset/empty environment variables via graphql #3208

Closed
tnelson-doghouse opened this issue Jul 8, 2022 · 8 comments
Closed

Support unset/empty environment variables via graphql #3208

tnelson-doghouse opened this issue Jul 8, 2022 · 8 comments

Comments

@tnelson-doghouse
Copy link

Is your feature request related to a problem? Please describe.

We sometimes (currently for testing purposes) want to override an environment variable from the codebase with an empty OR unset value (eg. to temporarily turn off basic auth).

Describe the solution you'd like

I'd like the Lagoon API to support unsetting environment variables (with the assumption that this would also become available via the lagoon-cli, where --value '' would set the empty value, and maybe --unset would unset the value, but that's a separate issue).

In discussions with Ben Jackson surrounding this, it seems like, if we also made the unsetting of environment variables also delete the configmap entry, that would solve problems for some other people.

Describe alternatives you've considered
The current workaround is to unset these in the codebase.

@shreddedbacon
Copy link
Member

There is already a bug uselagoon/build-deploy-tool#136 in which deleting a variable from the Lagoon API, it is not cleaned up from the configmap, this is part of the issue. Normally this has been done the opposite way, people set these variables in the API and then remove them from the environment when required. You're doing it the opposite way, which to me

The docs show the hierarchy of variables, so the empty value in the API should be made available in the pod if you're using the .lagoon.x, .env, or .env.x files see the entrypoint script, and it should overwrite what is in the local files. Currently the API seems to allow for empty variables to be set (but the CLI does not allow this, we can fix the CLI to allow empty values then).

        "id": 30,
        "envVariables": [
          {
            "name": "EMPTY_VARIABLE",
            "value": ""
          }
        ]

One concern would be that setting the value to empty may not solve this for all cases, and if we were to implement some sort of unset feature, this would need to then dig into the entrypoint script to be able to unset values that are defined in there. So only users of newer images would be able to leverage this.

@shreddedbacon
Copy link
Member

@tnelson-doghouse I released lagoon-cli v0.13.0 which allows for empty variables to be set in the API now.

Will leave this open though to discuss the unset functionality though

@tobybellwood
Copy link
Member

A combintaion of allowing '' and the CLI/UI updates should resolve this more easily

@tnelson-doghouse
Copy link
Author

Great, thanks!

Just so I have a clear picture of what's going on:

  • I see that I can set a variable to '' in the CLI, and it shows it as '-', which I assume means that it's empty and the CLI is just representing it that way.
  • I'm not seeing a way of overriding it to be just unset

I'm currently happy either way, but just wanted to be sure where we're at.

@shreddedbacon
Copy link
Member

shreddedbacon commented Oct 30, 2023

Yes - showing in the output of the cli is just a symptom of the way the table output works, it shouldn't actually show - so we should fix that. (uselagoon/lagoon-cli#299)

We're working on fixing the bug in Lagoon that deletes removed variables from the configmap, but there is still some work on that front.

As for unset, I don't really see any value in this once the delete from configmap is resolved. Unless you have better information about what you think it would solve?

@tnelson-doghouse
Copy link
Author

Only to override variables from the codebase to not exist in Lagoon, but not a high priority at all. Setting them as blank should be sufficient in most cases :) .

@shreddedbacon
Copy link
Member

Right, that was in case you had something in a .env file that you specifically wanted undefined, iirc?

@tnelson-doghouse
Copy link
Author

Yes, but it's not something I've seen a need for since I submitted the original ticket, so nice to have, but not at all a high priority.

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

No branches or pull requests

3 participants