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

[Feature] Support environment variables #5

Open
alexellis opened this issue Sep 6, 2022 · 14 comments · May be fixed by #7
Open

[Feature] Support environment variables #5

alexellis opened this issue Sep 6, 2022 · 14 comments · May be fixed by #7

Comments

@alexellis
Copy link
Owner

Inspired by the Kubernetes spec, support environment variables.

I think run-job should remain as simple as possible, but these are normally required for many kinds of container.

K8s format

    env:
    - name: DEMO_GREETING
      value: "Hello from the environment"
    - name: DEMO_FAREWELL
      value: "Such a sweet sorrow"

I'd be tempted to use the OpenFaaS format, but the K8s style may be easier for people migrating tasks.

environment:
  DEMO_FAREWELL: "Such a sweet sorrow"

By running the env built-in shell command in alpine:3.16, we can see whether the inputs have been received.

@Jasstkn
Copy link
Contributor

Jasstkn commented Sep 6, 2022

Hey. I would suggest to stick with Kubernetes specification because some people may want to do the following

kubectl get -oyaml job curl > job.yaml
run-job -f job.yaml

To do so seamlessly, it requires to be fully compatible with K8S spec.

@alexellis
Copy link
Owner Author

Hi @Jasstkn

This specifically will not work as expected, because the YAML format is different and much simpler:

kubectl get -oyaml job curl > job.yaml
run-job -f job.yaml

But they could do kubectl get -o yaml then copy/paste in the "env" section.

Would you like to add support for environment variables using the env format from the above?

@Jasstkn
Copy link
Contributor

Jasstkn commented Sep 6, 2022

Hi @Jasstkn

This specifically will not work as expected, because the YAML format is different and much simpler:

kubectl get -oyaml job curl > job.yaml
run-job -f job.yaml

But they could do kubectl get -o yaml then copy/paste in the "env" section.

Would you like to add support for environment variables using the env format from the above?

Ah, yes, you're right! Yep, I would like to implement this feature as well.

@clrxbl
Copy link

clrxbl commented Sep 6, 2022

Would be great to have support for this & envFrom. One of my use cases for run-job would be to run Flyway for SQL database migrations, and it would be nice to use envFrom to read from an existing secret.

@alexellis
Copy link
Owner Author

alexellis commented Sep 6, 2022

@clrxbl what data do you need for your envFrom use-cases?

If it's for secrets, we would also need to add secret support.

In OpenFaaS we do this by convention. Specify a list of secret names with secrets: and then the data is mounted in a projected volume under /var/openfaas/secrets/NAME/

That makes them quite convenient to read in bash / code.

@clrxbl
Copy link

clrxbl commented Sep 6, 2022

@clrxbl what data do you need for your envFrom use-cases?

I simply use envFrom to read from a single Secret containing environment variables for e.g. database authentication.

@alexellis
Copy link
Owner Author

Secret support seems reasonable for connecting to databases/APIs etc.

faas-cli list/describe etc instance also require faas-cli login first.

@Jasstkn
Copy link
Contributor

Jasstkn commented Oct 4, 2022

@clrxbl hello. We are in the discussion of what is the best way to implement this feature. Could you tell me what you feel is more convenient for you: the same approach as in the Kubernetes Job manifest or a more compact way with just a bare minimum and all parsing done under the hood?

FYI @alexellis

@clrxbl
Copy link

clrxbl commented Oct 4, 2022

@clrxbl hello. We are in the discussion of what is the best way to implement this feature. Could you tell me what you feel is more convenient for you: the same approach as in the Kubernetes Job manifest or a more compact way with just a bare minimum and all parsing done under the hood?

FYI @alexellis

It would be great to have it use the same approach as in the Kubernetes Job manifest as it allows you to refer to environment variables located in secrets.

@Jasstkn
Copy link
Contributor

Jasstkn commented Oct 4, 2022

@clrxbl hello. We are in the discussion of what is the best way to implement this feature. Could you tell me what you feel is more convenient for you: the same approach as in the Kubernetes Job manifest or a more compact way with just a bare minimum and all parsing done under the hood?
FYI @alexellis

It would be great to have it use the same approach as in the Kubernetes Job manifest as it allows you to refer to environment variables located in secrets.

You can take a look how I implemented it in the PR: #7 and also our discussion with @alexellis about possible improvements there: #7 (comment). I'm not sure which one is better.

@clrxbl
Copy link

clrxbl commented Oct 4, 2022

@clrxbl hello. We are in the discussion of what is the best way to implement this feature. Could you tell me what you feel is more convenient for you: the same approach as in the Kubernetes Job manifest or a more compact way with just a bare minimum and all parsing done under the hood?
FYI @alexellis

It would be great to have it use the same approach as in the Kubernetes Job manifest as it allows you to refer to environment variables located in secrets.

You can take a look how I implemented it in the PR: #7 and also our discussion with @alexellis about possible improvements there: #7 (comment). I'm not sure which one is better.

Looking at #7
IMO, it would be better to stick to how Kubernetes' formatting does it currently with env & envFrom.

@alexellis
Copy link
Owner Author

Could you give an example of how you think the YAML would look?

@clrxbl
Copy link

clrxbl commented Oct 4, 2022

Could you give an example of how you think the YAML would look?

name: checker
image: ghcr.io/openfaas/config-checker:latest
namespace: openfaas
sa: openfaas-checker
env:
  - name: "KEY"
    value: "value"
envFrom:
  - secretRef:
      name: "secret-name"

@Jasstkn
Copy link
Contributor

Jasstkn commented Oct 15, 2022

@alexellis hey. what do you think about the example above?

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 a pull request may close this issue.

3 participants