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

Enable users to inject "all" binding information as environment variables. #136

Open
sbose78 opened this issue Nov 5, 2020 · 9 comments
Milestone

Comments

@sbose78
Copy link
Contributor

sbose78 commented Nov 5, 2020

The spec has strong opinions on projecting binding information as "files". There's no way for a user of ServiceBinding to express the desire to Get me everything as environment variables.

While I understand the rationale behind everything being made available as "files" 📁 , a lot of app developers use frameworks/languages which are very sticky towards "environment variables" example, Golang devs.

I would propose we have a new field in the ServiceBinding CRD/API called

  • .spec.bindAs: files|env or
  • .spec.bindAsFiles: true or
  • .spec.bindAsEnv : true

In other words, it would be good to have something that would enable ServiceBinding users to have everything as environment variables too, instead of having to express the desire for the same with individual keys.

@sbose78 sbose78 changed the title Enable users to ask for having "all" binding information as environment variables. Enable users to inject "all" binding information as environment variables. Nov 5, 2020
@sbose78
Copy link
Contributor Author

sbose78 commented Nov 5, 2020

FYI: Service Binding operator is already implementing this. ( some of it planned, not done yet )

  • A global switch in the ServiceBinding CR
  • A switch specific to backing services in the ServiceBinding CR
  • A switch specific to fields in a backing service in the backing service's CR / CRD

@scothis
Copy link
Contributor

scothis commented Nov 5, 2020

Do you have a proposal for how bindAs: env should behave?

@sbose78
Copy link
Contributor Author

sbose78 commented Nov 11, 2020

bindAs: env should make all binding secret contents available in the workload, and possibly have a reasonable prefix ( name of the ServiceBinding ) to keep the injected environment variables unique enough for the container.

@navidsh
Copy link
Contributor

navidsh commented Nov 24, 2020

Hi @sbose78, we discussed this today during the community call and based on the conversation, would you be able to provide the following?

  • some details on the user story
  • some details on how the conversion from key/value entries ("Each key must consist of alphanumeric characters, '-', '_' or '.'") in the secret to environment variables

@sbose78
Copy link
Contributor Author

sbose78 commented Dec 15, 2020

Sorry, missed this message, I will @navidsh !

@sbose78
Copy link
Contributor Author

sbose78 commented Jan 13, 2021

For applications to discover, we could encourage use of an environment variable "SERVICE_BINDING_ENV_MAP" which would tell the runtime which env vars are of interest.

@sbose78
Copy link
Contributor Author

sbose78 commented Jan 13, 2021

some details on how the conversion from key/value entries ("Each key must consist of alphanumeric characters, '-', '_' or '.'") in the secret to environment variables

  • All alphabets would be uppercase.
  • Special characters would be converted to "_".

@baijum
Copy link
Contributor

baijum commented Jun 16, 2021

I think for the 1.0 release, we should proceed with the current approach of configuration as files. And there is limited support for creating environment variables pointing to the keys in the Secret resource. There are pros and cons to both approaches. Post 1.0 release, if the requirement comes up strongly from the community, we can revisit this issue.

Meanwhile, I was thinking of creating "environment variable transformation rules" to construct environment variables from the Secret resource entries. We could add an optional section like this:

envKeyTransformationRules:
  prefix: "STAGE_" # default empty string
  suffix: "_V2" # default empty string
  upperCase: true # make all character upper case
  replace:  # default empty array, replace the left character with the right one
    - ".": "_"
    - "-": "_"

If at least one rule exists, there MUST be environment variables corresponding to all the Secret entries. The transformation rules MUST NOT be applied to the environment variables defined in the .spec.env. The environment variable defined in .spec.env MUST overrides any environment variable constructed through the transformation rule.

Note: This option is not going to disable projecting files. That makes the change backward compatible.

@baijum
Copy link
Contributor

baijum commented Jun 28, 2021

One advantage of using file over environment variable is that mounted Secrets are updated automatically. The same benefit is not there for environment variable without restarting the container. To really make use of this behavior, the application should have some retry logic (probably with exponential backoff) implemented.

Note: this advantage completely depends on the implementation. The current spec is not making a mandate or recommendation to directly use the original Secret from Provisioned Service or Direct Secret Reference.

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

5 participants