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

default sourceSecret admission plugin #11383

Closed
wants to merge 5 commits into from

Conversation

mjudeikis
Copy link
Contributor

POC for default build secret for build.

2 outstanding questions:
first: if I add defualt secret, build fails, because secret is not mounted. How and where secret get mounted as volumes?
second: how I can initiate k8s client to list all available secrets in the project? I would like to check if secret exist before setting it.

previous pull request where we discussed this one:
#11275
@jim-minter @bparees @liggitt @csrwng

@mjudeikis mjudeikis changed the title add poc for default secret default sourceSecret admission plugin Oct 16, 2016
@@ -130,6 +141,18 @@ func (a *buildDefaults) applyBuildDefaults(build *buildapi.Build) {
build.Spec.Source.Git.NoProxy = &t
}
}

//apply default source secret if one set after all validation
//BUG: build fails because secret is not mounted... where to mount it?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does the secret exist in the project doing the build? that should be all that's required....

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checking (maybe typo during test or smth).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

apparently im doing something wrong:

[mangis@mj amd64]$ oc project --config=config/master/admin.kubeconfig
Using project "test" on server "https://192.168.1.116:8443".

secret :

[mangis@mj amd64]$ oc get secrets --config=config/master/admin.kubeconfig
NAME                       TYPE                                  DATA      AGE
scmsecret                  Opaque                                1         10m

after build is executed:

error: cannot setup source secret: no auth handler was found for secrets in /tmp/tmpsecret195217705

and pod describe does not show mounted secret:

oc describe pod ruby-sample-build-2-build --config=config/master/admin.kubeconfig
Name:           ruby-sample-build-2-build
Namespace:      test
Security Policy:    privileged
Node:           mj/192.168.1.116
Start Time:     Mon, 17 Oct 2016 19:47:50 +0100
Labels:         openshift.io/build.name=ruby-sample-build-2
Status:         Failed
IP:         172.17.0.2
Controllers:        <none>
Containers:
  sti-build:
    Container ID:   docker://3cd0a689909b5f0f9fad3f8c7f24ae4ecc4d755b76315039d689de6adaa63b66
    Image:      openshift/origin-sti-builder:v1.4.0-alpha.0
    Image ID:       docker://sha256:70676b107f3957c3bf8913f55e06a0385026e632cf998c78f5e781159ab870ef
    Port:       
    Args:
      --loglevel=0
    State:      Terminated
      Reason:       Error
      Exit Code:    1
      Started:      Mon, 17 Oct 2016 19:47:54 +0100
      Finished:     Mon, 17 Oct 2016 19:47:54 +0100
    Ready:      False
    Restart Count:  0
    Volume Mounts:
      /var/run/docker.sock from docker-socket (rw)
      /var/run/secrets/kubernetes.io/serviceaccount from builder-token-u01il (ro)
      /var/run/secrets/openshift.io/push from builder-dockercfg-tvzes-push (ro)
    Environment Variables:
      BUILD:    {"kind":"Build","apiVersion":"v1","metadata":{"name":"ruby-sample-build-2","namespace":"test","selfLink":"/oapi/v1/namespaces/test/builds/ruby-sample-build-2","uid":"348d5e97-949a-11e6-b21f-507b9dbf3051","resourceVersion":"2362","creationTimestamp":"2016-10-17T18:47:50Z","labels":{"app":"ruby-helloworld-sample","buildconfig":"ruby-sample-build","name":"ruby-sample-build","openshift.io/build-config.name":"ruby-sample-build","openshift.io/build.start-policy":"Serial","template":"application-template-stibuild"},"annotations":{"openshift.io/build-config.name":"ruby-sample-build","openshift.io/build.number":"2"}},"spec":{"serviceAccount":"builder","source":{"type":"Git","git":{"uri":"https://github.com/openshift/ruby-hello-world.git","httpProxy":"http://my.proxy:8080"},"sourceSecret":{"name":"scmsecret"}},"strategy":{"type":"Source","sourceStrategy":{"from":{"kind":"DockerImage","name":"centos/ruby-22-centos7@sha256:bab948dcca9f9f86014a25c0e0020438047bbed235cfbc5cfc375922ec04ddf3"},"env":[{"name":"EXAMPLE","value":"sample-app"}]}},"output":{"to":{"kind":"DockerImage","name":"172.30.161.64:5000/test/origin-ruby-sample:latest"},"pushSecret":{"name":"builder-dockercfg-tvzes"}},"resources":{},"postCommit":{"args":["bundle","exec","rake","test"]},"triggeredBy":[{"message":"Manually triggered"}]},"status":{"phase":"New","outputDockerImageReference":"172.30.161.64:5000/test/origin-ruby-sample:latest","config":{"kind":"BuildConfig","namespace":"test","name":"ruby-sample-build"}}}

      SOURCE_REPOSITORY:    https://github.com/openshift/ruby-hello-world.git
      SOURCE_URI:       https://github.com/openshift/ruby-hello-world.git
      ORIGIN_VERSION:       v1.4.0-alpha.0+9a0f45b-dirty
      ALLOWED_UIDS:     1-
      DROP_CAPS:        KILL,MKNOD,SETGID,SETUID,SYS_CHROOT
      PUSH_DOCKERCFG_PATH:  /var/run/secrets/openshift.io/push
Conditions:
  Type      Status
  Initialized   True 
  Ready     False 
  PodScheduled  True 
Volumes:
  docker-socket:
    Type:   HostPath (bare host directory volume)
    Path:   /var/run/docker.sock
  builder-dockercfg-tvzes-push:
    Type:   Secret (a volume populated by a Secret)
    SecretName: builder-dockercfg-tvzes
  builder-token-u01il:
    Type:   Secret (a volume populated by a Secret)
    SecretName: builder-token-u01il
QoS Tier:   BestEffort
Events:
  FirstSeen LastSeen    Count   From            SubobjectPath           Type        Reason      Message
  --------- --------    -----   ----            -------------           --------    ------      -------
  7m        7m      1   {default-scheduler }                    Normal      Scheduled   Successfully assigned ruby-sample-build-2-build to mj
  7m        7m      1   {kubelet mj}        spec.containers{sti-build}  Normal      Pulled      Container image "openshift/origin-sti-builder:v1.4.0-alpha.0" already present on machine
  7m        7m      1   {kubelet mj}        spec.containers{sti-build}  Normal      Created     Created container with docker id 3cd0a689909b; Security:[seccomp=unconfined]
  7m        7m      1   {kubelet mj}        spec.containers{sti-build}  Normal      Started     Started container with docker id 3cd0a689909b

but source secret is available in description:

sourceSecret":{"name":"scmsecret"}}

any hints?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Mangirdaz I tried with a build locally and cannot reproduce what you're seeing. In my case, the secret gets mounted in the pod when I specify it in the build.spec.sourceSecret field. What version of the server are you using?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

newest cut from master (1.4.0.x). are you setting just a name?
build.spec.sourceSecret.Name=scmsecret ?

@bparees
Copy link
Contributor

bparees commented Oct 17, 2016

just a heads up, this is going to need to be rebased/reworked after #11380 merges. (Assuming we decide we do want to add secrets as a defaultable thing)

//second option is global config.
ns, err := a.cache.GetNamespace(build.Namespace)
if err == nil {
if contains(ns.Annotations, "openshift.io/sourceSecret") {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this so that project admins can customize the default secret for their namespace? normal project admins won't have write permission to these annotations

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be for the case where you have multiple project templates or as cluster admin you want to setup something else for a specific project.

@mjudeikis
Copy link
Contributor Author

Question, How I can get k8s client in the code to read available secrets for the project?

@csrwng
Copy link
Contributor

csrwng commented Oct 17, 2016

@Mangirdaz the k8s client is available when the plugin is registered:
https://github.com/openshift/origin/pull/11383/files#diff-9a575b8566b583a7dc6bf9764beaf0aeL18

It can be passed to the constructor of the plugin so you can use it later.
However, for something like secrets, there may already be a cache you can use to get the list, @liggitt ?

@liggitt
Copy link
Contributor

liggitt commented Oct 17, 2016

I still think it's weird to have built-in support for a fixed secret name without built in support for ensuring that secret exists. I also am not a fan of "include a secret named foo by default if it exists"... if I create a secret with that name in a namespace, I would not expect my builds to suddenly start picking up that secret

@mjudeikis
Copy link
Contributor Author

mjudeikis commented Oct 17, 2016

trust me I'm not too. But consider situation. We run Openshift in organisation where users are not so smart, adoption rate is slow and everything is oversecure. This means our git is secure, our artifact repo is secure too.

Having this all in mind, I would like to have "hello world" app via S2I builder from the UI, or oc new-app example. But I just can't, because I cant use secure GIT. So learning curve become very steam, as User have to learn what is secrets, what is templates, and how to use OC client from the day 1. This brings a lot of complexity for platform adoption.

I think @csrwng can give more insights with what kind of "magic" we have to deal for the sake of the platform as he seen things with his own eyes ;)

@liggitt
Copy link
Contributor

liggitt commented Oct 17, 2016

This means our git is secure, our artifact repo is secure too.

I'm surprised a project template that sets up a globally shared secret with credentials to git/artifact repo is acceptable in that environment

@csrwng
Copy link
Contributor

csrwng commented Oct 17, 2016

@liggitt my understanding is that the secret name is global, but each project has its own actual secret.

@mjudeikis
Copy link
Contributor Author

Yes. Initially we set secret which has access to "example" project, and user can test platform, play around, and learn first step. And when its ready, user can create their own secrets and point to their own secure projects.

@csrwng
Copy link
Contributor

csrwng commented Oct 18, 2016

if I create a secret with that name in a namespace, I would not expect my builds to suddenly start picking up that secret

Remember that this is an option you're going to pick for your environment as a cluster admin. If it doesn't make sense for your environment, you shouldn't set this default.

@liggitt
Copy link
Contributor

liggitt commented Oct 18, 2016

but this interacts with user-created things in unpredictable ways. other cluster-admin-created things always apply (like build overrides or project node selector)

@csrwng
Copy link
Contributor

csrwng commented Oct 18, 2016

@Mangirdaz this PR #11383 added support for entering secret info when creating from an image wouldn't that be sufficient for your use case?

@csrwng
Copy link
Contributor

csrwng commented Oct 18, 2016

@csrwng
Copy link
Contributor

csrwng commented Oct 18, 2016

Please see URL to web console repo

@mjudeikis
Copy link
Contributor Author

@csrwng , yes and no. It good, as its tells user he need something. But secret still need to exist before referring it. This means going and learning how to do it as a first step.

@csrwng
Copy link
Contributor

csrwng commented Oct 18, 2016

You can create the secret as part of the flow though

@mjudeikis
Copy link
Contributor Author

Kinda yes. So does this mean we want to ditch this one? :)

@csrwng
Copy link
Contributor

csrwng commented Oct 18, 2016

If it meets your needs, then yes :) It'd be good for you try it out and see if it does.

@csrwng
Copy link
Contributor

csrwng commented Oct 24, 2016

@Mangirdaz, after chatting with @bparees about this some more, the way we want to move forward is to handle default secrets and such through UI defaults rather than something on the server. As you know, we have a card to handle this in new-app: https://trello.com/c/iotn8FCo/873-8-make-new-app-generate-based-on-skeleton-definition-evg and we would need to follow suit in the web console.

@mjudeikis
Copy link
Contributor Author

@csrwng , in this case f this fine with you I will be closing this one, and waiting for "Skeleton to be implemented" , I seen it was in backlog. Is where any change it will get prioritized and moved up?

  • stuff to refer secret in UI will help us too for a time being.

@openshift-bot
Copy link
Contributor

Origin Action Required: Pull request cannot be automatically merged, please rebase your branch from latest HEAD and push again

@openshift-bot openshift-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 24, 2016
@csrwng
Copy link
Contributor

csrwng commented Oct 24, 2016

Is where any change it will get prioritized and moved up?

Definitely we'll discuss with team in the next iteration planning.

And yes, go ahead and close this one.

@mjudeikis mjudeikis closed this Oct 24, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/build needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. priority/P2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants