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

Show warning when kubectl set env from secret leads to uppercase of the secret keys #1171

Closed
ThuF opened this issue Jan 14, 2022 · 17 comments · Fixed by kubernetes/kubernetes#107934
Assignees
Labels
kind/feature Categorizes issue or PR as related to a new feature. priority/backlog Higher priority than priority/awaiting-more-evidence. triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@ThuF
Copy link

ThuF commented Jan 14, 2022

What happened?

I have a Secret created by the following command:

kubectl -n <namespace-name> create secret generic xsuaa-credentials \
--from-literal=url='<xsuaa-url>' \
--from-literal=clientid='<xsuaa-clientid>' \
--from-literal=clientsecret='<xsuaa-clientsecret>' \
--from-literal=verificationkey='<xsuaa-verificationkey>' \
--from-literal=xsappname='<xsuaa-xsappname>'

Note: The secret keys are in lowercase.

I'm binding the secret to a Deployment with the following command:

kubectl -n <namespace-name> set env --from=secret/xsuaa-credentials deployment/xsk

When describing the deployment I get the following output:

Name:                   xsk
Namespace:              xsk-demo-sdi
CreationTimestamp:      Fri, 14 Jan 2022 18:27:12 +0200
Labels:                 <none>
Annotations:            deployment.kubernetes.io/revision: 2
                        kubectl.kubernetes.io/last-applied-configuration:
                          {"apiVersion":"apps/v1","kind":"Deployment","metadata":{"annotations":{},"name":"xsk","namespace":"xsk-demo-sdi"},"spec":{"replicas":1,"se...
Selector:               app=xsk
Replicas:               1 desired | 1 updated | 1 total | 1 available | 0 unavailable
StrategyType:           RollingUpdate
MinReadySeconds:        0
RollingUpdateStrategy:  25% max unavailable, 25% max surge
Pod Template:
  Labels:  app=xsk
  Containers:
   xsk:
    Image:      dirigiblelabs/xsk-kyma:latest
    Port:       8080/TCP
    Host Port:  0/TCP
    Environment:
      URL:                      <set to the key 'url' in secret 'xsuaa-credentials'>              Optional: false
      VERIFICATIONKEY:          <set to the key 'verificationkey' in secret 'xsuaa-credentials'>  Optional: false
      XSAPPNAME:                <set to the key 'xsappname' in secret 'xsuaa-credentials'>        Optional: false
      CLIENTID:                 <set to the key 'clientid' in secret 'xsuaa-credentials'>         Optional: false
      CLIENTSECRET:             <set to the key 'clientsecret' in secret 'xsuaa-credentials'>     Optional: false

Where the environment variables are in uppercase:

  • clientid → CLIENTID
  • clientsecret → CLIENTSECRET
  • verificationkey → VERIFICATIONKEY
  • xsappname → XSAPPNAME

What did you expect to happen?

The environment variables to be in lowercase, as they are defined in the secret.

How can we reproduce it (as minimally and precisely as possible)?

Shown in the description.

Anything else we need to know?

No response

Kubernetes version

$ kubectl version
Client Version: version.Info{Major:"1", Minor:"14", GitVersion:"v1.14.1", GitCommit:"b7394102d6ef778017f2ca4046abbaa23b88c290", GitTreeState:"clean", BuildDate:"2019-04-19T22:12:47Z", GoVersion:"go1.12.4", Compiler:"gc", Platform:"darwin/amd64"}
Server Version: version.Info{Major:"1", Minor:"21", GitVersion:"v1.21.6", GitCommit:"d921bc6d1810da51177fbd0ed61dc811c5228097", GitTreeState:"clean", BuildDate:"2021-10-27T17:44:26Z", GoVersion:"go1.16.9", Compiler:"gc", Platform:"linux/amd64"}

Cloud provider

AWS via Gardener

OS version

# On Linux:
$ cat /etc/os-release
# paste output here
$ uname -a
# paste output here

# On Windows:
C:\> wmic os get Caption, Version, BuildNumber, OSArchitecture
# paste output here

Install tools

Container runtime (CRI) and and version (if applicable)

Related plugins (CNI, CSI, ...) and versions (if applicable)

@ThuF ThuF changed the title Jan 14, 2022
@neolit123
Copy link
Member

/sig cli

@Shubham82
Copy link
Contributor

Hi @ThuF
I think this is due to the keyToEnvName(key string) string function mentioned here which Uppercase every Environment key Name. So due to this container env keys name is always in uppercase.
I don't think it is a bug but anyone from /sig cli can give more clarity on this, either we have to change it or not.

@deejross
Copy link
Contributor

Platforms can treat environment variable name case-sensitivity differently. This is why it's been accepted as good practice to use all uppercase environment variable names. The Google Style Guide recommends this as well: https://google.github.io/styleguide/shellguide.html#s7.3-constants-and-environment-variable-names

@Shubham82
Copy link
Contributor

Thanks, @deejross for this information.

@ThuF
Copy link
Author

ThuF commented Jan 19, 2022

@Shubham82, @deejross, thanks for the comments!

I totally agree that's a good practice to use uppercase environment variables, but there are cases where lowercase/camelcase/etc. format of the keys is expected, due to legacy reasons or simply limitation of the platform. For example, you could take a look at https://kyma-project.io/ and its implementation as part of the SAP BTP, Kyma environment. As described in this blog a Authorization & Trust Management service instance is created, which contains only lowercase keys (that's part of the implementation of the service instance binding) and that can't be modified. So a Secret containing these lower keys is created (as part of the Service Instance Binding) and can't be updated.

What I have as a workaround is to edit the Deployment and update the case sensitive environment variables.

@sftim
Copy link
Contributor

sftim commented Jan 20, 2022

At a minimum, we should clearly document this behavior (consider opening an issue against k/website to request that).

It would also be useful to have kubectl emit a warning that it has rewritten the keys to uppercase.

@sftim
Copy link
Contributor

sftim commented Jan 20, 2022

/remove-kind bug
(triage is unclear at this time)

@jlsong01
Copy link
Contributor

@sftim Hey, I would like to add a warning in case the key rewritten to uppercase.

Since it has not been triaged, could I go for it now?

@sftim
Copy link
Contributor

sftim commented Jan 30, 2022

/triage accepted

@sftim
Copy link
Contributor

sftim commented Jan 30, 2022

PRs welcome!
/language en

@k8s-ci-robot
Copy link
Contributor

@sftim: The label(s) language/en cannot be applied, because the repository doesn't have them.

In response to this:

PRs welcome!
/language en

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@sftim
Copy link
Contributor

sftim commented Jan 30, 2022

Oh, sorry. I was thinking this was the documentation issue. I'm the wrong person to ask about adding the warning to kubectl; that's a decision for SIG CLI.
/remove-triage accepted

@k8s-ci-robot
Copy link
Contributor

@ThuF: This issue is currently awaiting triage.

If a SIG or subproject determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@sftim
Copy link
Contributor

sftim commented Jan 30, 2022

/transfer kubectl

@k8s-ci-robot k8s-ci-robot transferred this issue from kubernetes/kubernetes Jan 30, 2022
@k8s-ci-robot k8s-ci-robot added the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Jan 30, 2022
@sftim
Copy link
Contributor

sftim commented Jan 30, 2022

/retitle Show warning when kubectl set env from secret leads to uppercase of the secret keys

/kind feature

@k8s-ci-robot k8s-ci-robot changed the title Kubectl set env from secret leads to uppercase of the secret keys Show warning when kubectl set env from secret leads to uppercase of the secret keys Jan 30, 2022
@k8s-ci-robot k8s-ci-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Jan 30, 2022
@eddiezane
Copy link
Member

At a minimum, we should clearly document this behavior (consider opening an issue against k/website to request that).

It would also be useful to have kubectl emit a warning that it has rewritten the keys to uppercase.

Agreed. We would accept a PR that adds a warning if the to upper is different from original.

@jlsong01 did you still want to take this one?

/triage accepted
/priority backlog

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. priority/backlog Higher priority than priority/awaiting-more-evidence. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Feb 2, 2022
@jlsong01
Copy link
Contributor

jlsong01 commented Feb 3, 2022

/assign
glad to take this one

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. priority/backlog Higher priority than priority/awaiting-more-evidence. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants