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

kn secret for managing secrets #287

Closed
duglin opened this issue Jul 23, 2019 · 27 comments · Fixed by #1791
Closed

kn secret for managing secrets #287

duglin opened this issue Jul 23, 2019 · 27 comments · Fixed by #1791
Labels
Epic Epics to group issues kind/feature New feature or request kind/proposal Issues or PRs related to proposals. triage/accepted Issues which should be fixed (post-triage)
Milestone

Comments

@duglin
Copy link
Contributor

duglin commented Jul 23, 2019

In what area(s)?

Classifications:
/kind good-first-issue
/kind feature
/kind proposal

Describe the feature:

With us on the verge of adding support for KnEventing, it seems that we should consider adding support for basic secret creation/management in kn. I say this because from a UX perspective it would be really nice if the user didn't have to switch between kn and kubectl just to do the basic KnEventing actions - like create a github event source. I like the goal of trying to keep them in the kn world as much as possible. Obviously, there will always be a balance we'll need to watch for, but I think some basic secret stuff makes sense.

We should look at what you can do with kubectl to create secrets. Passing in name/value pairs is obvious, but reading them from files on disk is another common (and probably safer) thing to support as well.

We could then also consider adding secret volume mounting in the service cmd.

We might want to think about configMaps at some point too - but my more immediate concern is secrets because once Eventing support is added I think that'll be a more obvious gap in our feature set.

@duglin duglin added the kind/feature New feature or request label Jul 23, 2019
@knative-prow-robot knative-prow-robot added good first issue Denotes an issue ready for a new contributor. kind/proposal Issues or PRs related to proposals. labels Jul 23, 2019
@rhuss
Copy link
Contributor

rhuss commented Jul 24, 2019

I don't think that Secret is part of the Knative API contract (i.e. there is no entry for /apis/v1/namespaces/<namespace>/secret/<name> in https://github.com/knative/serving/blob/master/docs/spec/spec.md).

We should first get this supported in the API so that non-Kubernetes based clients can use that as well. I'm afraid, it's not something the client could do right now.

@mattmoor
Copy link
Member

mattmoor commented Aug 5, 2019

FWIW, we're moving most of the serving specification stuff to: https://github.com/knative/docs/blob/master/docs/serving/spec/knative-api-specification-1.0.md

The omission of Secrets (or ConfigMaps) there was not intended to mean "this cannot be in clients", I believe it was simply "these aren't our resources to specify", or "Gee, I wish K8s had a more formal API specification." 😄

I think it should be acceptable for us to reference from our clients resources that are referenced from our API objects, including:

  1. Secrets
  2. ConfigMaps
  3. ServiceAccounts -- I think I'm more mixed about this (not sure if it fits the right persona for kn), but I also think the priority of this is much lower than the others, and so I'd rather not front-load an argument about it.

My $0.02

@rhuss
Copy link
Contributor

rhuss commented Aug 5, 2019

  • Secrets
  • ConfigMaps
  • ServiceAccounts -- I think I'm more mixed about this (not sure if it fits the right persona for kn), but I also think the priority of this is much lower than the others, and so I'd rather not front-load an argument about it.

@mattmoor would listing of namespaces also fall in this kind of category ? (as we would have to find out the namespaces available so that a user knows wheter to put her services into ?)

@rhuss
Copy link
Contributor

rhuss commented Aug 5, 2019

@mattmoor would it make sense to collect an explicit list of those resource types which are referenced by Knative resources and add them to the spec to have more clarity ? And also, whether this access should be supported as read-write (e.g. ConfigMap, Secret) or read-only (Namespace) ?

@mattmoor
Copy link
Member

mattmoor commented Aug 5, 2019

Namespace is an interesting case since it straddles what I generally see as a perceived line between operators and developers (or tenants).

If we see kn as a tool catering to developers, then I think read access to Namespace makes sense, but maybe not write. That said, in general listing namespaces is a potential privacy leak on shared clusters, which I don't think Kube has a good story for: e.g. "list the cluster-scoped things to which I have access?". I wonder what Openshift tooling does here? cc @pmorie @markusthoemmes @bbrowning

For namespaced resources, which all of Secrets, ConfigMaps, and ServiceAccounts are, then I think read/write makes some sense. That said, read-only may be the sweet spot for ServiceAccount because "write" is mainly interesting if you also start to creep in Roles and RoleBindings, ...

So I think (again, my $0.02) something like the following makes sense:

  • read/write: ConfigMap / Secret
  • read-only: Namespace / ServiceAccount

WDYT?

@duglin
Copy link
Contributor Author

duglin commented Aug 5, 2019

ServiceAccount falls into a different category to me too - but that's not to say I'd be against exposing them via kn at some point :-)

But Namespaces is a common thing people may create as a way to separate their apps. If an operator wants to limit who create them then they would do so via RBAC so kn doesn't need to get into the permission side of things. However, if RBAC is setup to allow this user to create a namespace then if the first step of their "create a new app"workflow is create a namespace, but they can't do it via kn (and they need to use kubectl) then I feel like we've let them down from a UX perspective.

@markusthoemmes
Copy link
Contributor

Especially on namespaces: Are we going to create any additional value here? Or are we literally replacing kubectl create namespace XXX with kn namespaces create XXX.

If it's the latter I feel like it's not really worth getting into that business and just tell people to use kubectl. That also eliminates potential confusion on whether or not Kubernetes namespace == Knative namespace and vice versa.

@sixolet
Copy link
Contributor

sixolet commented Aug 5, 2019 via email

coryrc pushed a commit to coryrc/client that referenced this issue May 14, 2020
* add knative testgrid to README

* fix the typo
@github-actions
Copy link

This issue is stale because it has been open for 90 days with no
activity. It will automatically close after 30 more days of
inactivity. Reopen the issue with /reopen. Mark the issue as
fresh by adding the comment /remove-lifecycle stale.

@github-actions github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 15, 2020
@duglin
Copy link
Contributor Author

duglin commented Oct 15, 2020

/remove-lifecycle statle

I'd like to revisit this one. I feel like thinking has shifted since this was first opened and we should consider expanding the list of objects that kn supports

@rhuss
Copy link
Contributor

rhuss commented Nov 26, 2020

So if we can agree that ConfigMap and Secret are part of Knative and can be fully managed (create/describe/list/delete/update), then I have no principal issue with that except maybe that we don't add any value of the kubectl management of those (which is quite feature rich). We would need to copy these over, so ideally we would just add a little shim and then call the kubectl implementation for managing those entities.

I would put them either top-level (kn configmap create, kn secret list) or one level below to not pollute the top-level namespace (e.g. kn util configmap create and kn util secret list).

What are the communities thoughts on the usefulness of such a command ?

@duglin
Copy link
Contributor Author

duglin commented Nov 26, 2020

I'd prefer top level entities so that all resources a user might need to manage are treated equally. Similar to how we have the eventing resources at the same level as the knService resources.

As for duplicating what kubectl does, from a UX perspective the more people can get their job done via one CLI the better the experience, IMO.

@github-actions github-actions bot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 23, 2021
@lamw
Copy link

lamw commented Feb 11, 2021

Great to see this idea being discussed! I recently came across this challenge as consumer of Knative and realized that we'd have to ask our end users to switch between kn and kubectl to handle arbitrary secrets/sensitive information that would can then be accessible within Knative service.

@rhuss Your proposal sounds great (prefer top level as well), how do we go about moving this forward?

@rhuss
Copy link
Contributor

rhuss commented Feb 15, 2021

Similar to how we have the eventing resources at the same level as the knService resources.

That's not true, for source we have everything one layer deeper kn source ping create, so there is already some 'ordering' level (having those top-level would have lead to kn ping-source create or kn apiserver-source create).

So from that pov, I wouldn't mind adding a kn config secret create or kn config cm create, also to make it clear that those are not key parts of Knative itself. But let's decide this when starting to implement it.

@rhuss
Copy link
Contributor

rhuss commented Feb 15, 2021

@rhuss Your proposal sounds great (prefer top level as well), how do we go about moving this forward?

The next step would be to think about the implementation of this feature and how it looks like from the CLI. For larger new features we should create a Feature Track document (like it is common for Knative serving), but for this one I would suggest that we just mimic the way how kubectl implements the secret/cm management. We should just decided whether we want to skip some of the more advanced features for a first step.

ConfigMap

Create a configmap based on a file, directory, or specified literal value.

 A single configmap may package one or more key/value pairs.

 When creating a configmap based on a file, the key will default to the basename of the file, and the value will default
to the file content.  If the basename is an invalid key, you may specify an alternate key.

 When creating a configmap based on a directory, each file whose basename is a valid key in the directory will be
packaged into the configmap.  Any directory entries except regular files are ignored (e.g. subdirectories, symlinks,
devices, pipes, etc).

Aliases:
configmap, cm

Examples:
  # Create a new configmap named my-config based on folder bar
  kubectl create configmap my-config --from-file=path/to/bar

  # Create a new configmap named my-config with specified keys instead of file basenames on disk
  kubectl create configmap my-config --from-file=key1=/path/to/bar/file1.txt --from-file=key2=/path/to/bar/file2.txt

  # Create a new configmap named my-config with key1=config1 and key2=config2
  kubectl create configmap my-config --from-literal=key1=config1 --from-literal=key2=config2

  # Create a new configmap named my-config from the key=value pairs in the file
  kubectl create configmap my-config --from-file=path/to/bar

  # Create a new configmap named my-config from an env file
  kubectl create configmap my-config --from-env-file=path/to/bar.env

Options:
      --allow-missing-template-keys=true: If true, ignore any errors in templates when a field or map key is missing in
the template. Only applies to golang and jsonpath output formats.
      --append-hash=false: Append a hash of the configmap to its name.
      --dry-run='none': Must be "none", "server", or "client". If client strategy, only print the object that would be
sent, without sending it. If server strategy, submit server-side request without persisting the resource.
      --field-manager='kubectl-create': Name of the manager used to track field ownership.
      --from-env-file='': Specify the path to a file to read lines of key=val pairs to create a configmap (i.e. a Docker
.env file).
      --from-file=[]: Key file can be specified using its file path, in which case file basename will be used as
configmap key, or optionally with a key and file path, in which case the given key will be used.  Specifying a directory
will iterate each named file in the directory whose basename is a valid configmap key.
      --from-literal=[]: Specify a key and literal value to insert in configmap (i.e. mykey=somevalue)
  -o, --output='': Output format. One of:
json|yaml|name|go-template|go-template-file|template|templatefile|jsonpath|jsonpath-as-json|jsonpath-file.
      --save-config=false: If true, the configuration of current object will be saved in its annotation. Otherwise, the
annotation will be unchanged. This flag is useful when you want to perform kubectl apply on this object in the future.
      --template='': Template string or path to template file to use when -o=go-template, -o=go-template-file. The
template format is golang templates [http://golang.org/pkg/text/template/#pkg-overview].
      --validate=true: If true, use a schema to validate the input before sending it

Usage:
  kubectl create configmap NAME [--from-file=[key=]source] [--from-literal=key1=value1] [--dry-run=server|client|none]
[options]

Secret

kubectl create secret generic --help
Create a secret based on a file, directory, or specified literal value.

 A single secret may package one or more key/value pairs.

 When creating a secret based on a file, the key will default to the basename of the file, and the value will default to
the file content. If the basename is an invalid key or you wish to chose your own, you may specify an alternate key.

 When creating a secret based on a directory, each file whose basename is a valid key in the directory will be packaged
into the secret. Any directory entries except regular files are ignored (e.g. subdirectories, symlinks, devices, pipes,
etc).

Examples:
  # Create a new secret named my-secret with keys for each file in folder bar
  kubectl create secret generic my-secret --from-file=path/to/bar

  # Create a new secret named my-secret with specified keys instead of names on disk
  kubectl create secret generic my-secret --from-file=ssh-privatekey=path/to/id_rsa
--from-file=ssh-publickey=path/to/id_rsa.pub

  # Create a new secret named my-secret with key1=supersecret and key2=topsecret
  kubectl create secret generic my-secret --from-literal=key1=supersecret --from-literal=key2=topsecret

  # Create a new secret named my-secret using a combination of a file and a literal
  kubectl create secret generic my-secret --from-file=ssh-privatekey=path/to/id_rsa --from-literal=passphrase=topsecret

  # Create a new secret named my-secret from an env file
  kubectl create secret generic my-secret --from-env-file=path/to/bar.env

Options:
      --allow-missing-template-keys=true: If true, ignore any errors in templates when a field or map key is missing in
the template. Only applies to golang and jsonpath output formats.
      --append-hash=false: Append a hash of the secret to its name.
      --dry-run='none': Must be "none", "server", or "client". If client strategy, only print the object that would be
sent, without sending it. If server strategy, submit server-side request without persisting the resource.
      --field-manager='kubectl-create': Name of the manager used to track field ownership.
      --from-env-file='': Specify the path to a file to read lines of key=val pairs to create a secret (i.e. a Docker
.env file).
      --from-file=[]: Key files can be specified using their file path, in which case a default name will be given to
them, or optionally with a name and file path, in which case the given name will be used.  Specifying a directory will
iterate each named file in the directory that is a valid secret key.
      --from-literal=[]: Specify a key and literal value to insert in secret (i.e. mykey=somevalue)
  -o, --output='': Output format. One of:
json|yaml|name|go-template|go-template-file|template|templatefile|jsonpath|jsonpath-as-json|jsonpath-file.
      --save-config=false: If true, the configuration of current object will be saved in its annotation. Otherwise, the
annotation will be unchanged. This flag is useful when you want to perform kubectl apply on this object in the future.
      --template='': Template string or path to template file to use when -o=go-template, -o=go-template-file. The
template format is golang templates [http://golang.org/pkg/text/template/#pkg-overview].
      --type='': The type of secret to create
      --validate=true: If true, use a schema to validate the input before sending it

Usage:
  kubectl create secret generic NAME [--type=string] [--from-file=[key=]source] [--from-literal=key1=value1]
[--dry-run=server|client|none] [options]

Use "kubectl options" for a list of global command-line options (applies to all commands).

Kubectl differentiates between three kind of secrets, I think we should only focus on the 'generic' secret (not tls secret, no docker secret)


The implementation itself should reuse as much as possible for the kubectl client library. The first step would be to analyse how kubectl does it and then we can reuse the same code here.

However, first of all we need to think where to put the command. I add a poll in the next two comment. This pool is open until the end of the week.

@lamw Would you be interested into implementing this feature ?

@rhuss
Copy link
Contributor

rhuss commented Feb 15, 2021

Option A:

Use a top-level command like in kn secret create and kn configmap create (with an alias kn cm create for managing secrets and configmaps.

Please vote with 👍 if you in favour of this option (don't vote down please)

@rhuss
Copy link
Contributor

rhuss commented Feb 15, 2021

Option B

Use a sub-level liks for eventing source to not pollute the top-level namespace and add some grouping for k8s related management commands (like e.g. we could for namespaces, too). Another argument would be that we don't manage Knative objects here. Like in kn config cm create or kn config secret create. The group-level name still needs then to be discussed (.e.g util, config, k8s, ...)

Please vote with 👍 if you in favour of this option (don't vote down please)

@rhuss
Copy link
Contributor

rhuss commented Feb 15, 2021

Please vote Option A or B until EOD PDT of February 19th.

@csantanapr
Copy link
Member

csantanapr commented Feb 15, 2021

Have a copy and support all options as kubectl create secret including registry creds.

alias “kn secret create”=”kubectl create secret”
alias “kn <cm | configMap> create”=”kubectl create cm”

@lamw
Copy link

lamw commented Feb 15, 2021

Would you be interested into implementing this feature ?

Thanks for the write-up @rhuss and no, I'm not a developer, so I wouldn't be able to help with the implementation but more than happy to help with the testing/consumption :)

@embano1
Copy link
Contributor

embano1 commented Feb 15, 2021

I think @csantanapr raises a good point: if we simply mimic kubectl behavior, there's probably not much gain for the end user. What is the UX we can provide on top which justifies a top-level secret | configmap command?

@rhuss
Copy link
Contributor

rhuss commented Feb 15, 2021

@embano1 the reasoning above was to add a kubectl feature 1:1 to allow for a single tool (kn) managing all aspects of Knative applications. No need for an installing kubectl in addition.

From this issue's description by @duglin

I say this because from a UX perspective it would be really nice if the user didn't have to switch between kn and kubectl just to do the basic KnEventing actions - like create a github event source. I like the goal of trying to keep them in the kn world as much as possible

If we can reuse kubectl commands by including those as dependencies (like we do for machine-readable output for kn service describe -o yaml) then it's just added convenience to the user.

@rhuss
Copy link
Contributor

rhuss commented Feb 15, 2021

Thanks for the write-up @rhuss and no, I'm not a developer, so I wouldn't be able to help with the implementation but more than happy to help with the testing/consumption :)

no worries, was just asking ;-) Anybody who is interested in implementing this feature, feel free to pick it up.

@github-actions
Copy link

This issue is stale because it has been open for 90 days with no
activity. It will automatically close after 30 more days of
inactivity. Reopen the issue with /reopen. Mark the issue as
fresh by adding the comment /remove-lifecycle stale.

@github-actions github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label May 17, 2021
@duglin
Copy link
Contributor Author

duglin commented May 17, 2021

/remove-lifecycle stale

@knative-prow-robot knative-prow-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label May 17, 2021
@github-actions
Copy link

This issue is stale because it has been open for 90 days with no
activity. It will automatically close after 30 more days of
inactivity. Reopen the issue with /reopen. Mark the issue as
fresh by adding the comment /remove-lifecycle stale.

@github-actions github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Aug 16, 2021
@rhuss rhuss added the triage/accepted Issues which should be fixed (post-triage) label Sep 1, 2021
@rhuss
Copy link
Contributor

rhuss commented Sep 1, 2021

/remove-lifecycle stale

@knative-prow-robot knative-prow-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Sep 1, 2021
@rhuss rhuss changed the title Consider basic kn secret command kn secret for managing secrets Jan 27, 2022
@rhuss rhuss added Epic Epics to group issues and removed type/epic labels Jan 31, 2022
@dsimansk dsimansk removed the good first issue Denotes an issue ready for a new contributor. label Aug 1, 2022
@dsimansk dsimansk added this to the Knative 1.10 milestone Mar 23, 2023
dsimansk added a commit to dsimansk/client that referenced this issue Nov 2, 2023
* [release-v1.10] Update Quarkus version in plugins to 2.13.8.SP3-redhat-00001

* Use S-O release branch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Epic Epics to group issues kind/feature New feature or request kind/proposal Issues or PRs related to proposals. triage/accepted Issues which should be fixed (post-triage)
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

10 participants