Rework Secrets in BuildConfig to use ObjectReference #2347
Conversation
|
Fixes: #2326 |
|
[test] |
|
continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin/2323/) |
|
@bparees @smarterclayton @liggitt PTAL I will do follow up (change ObjectReference to LocalObjectReference or whatever will land in upstream). |
|
lgtm. |
|
@bparees just realized that I have to do validation for this. |
mfojtik
force-pushed
the
secrets-references
branch
from
9fd2dbc to
1b752af
Compare
mfojtik
force-pushed
the
secrets-references
branch
from
1b752af to
aa4c2ea
Compare
|
@smarterclayton you mentioned a rebase. That would obviate the need for the upstream patch. ETA? |
| // the authentication for pulling the Docker images from the private Docker | ||
| // registries | ||
| PullSecretName string `json:"pullSecretName,omitempty"` | ||
| PullSecret *kapi.LocalObjectReference `json:"pullSecret,omitempty"` |
There was a problem hiding this comment.
Add the requirement on the type of Secrets that are allowed to the comment.
There was a problem hiding this comment.
good idea, I will also add description
|
I think the only thing I'm worried about is validating the secret (#2347 (comment) and #2347 (comment)). After you've got that, lgtm |
| // the base64 encoded credentials. Supported auth methods are: ssh-privatekey. | ||
| SourceSecretName string `json:"sourceSecretName,omitempty" description:"supported auth methods are: ssh-privatekey` | ||
| SourceSecret *kapi.LocalObjectReference `json:"sourceSecret,omitempty" description:"supported auth methods are: ssh-privatekey"` |
There was a problem hiding this comment.
I didn't think we could make changes like this... shouldn't this stay a string and be changed to a LocalObjectReference in conversion?
There was a problem hiding this comment.
It wasn't in long enough to qualify for API stability guarantees
On May 20, 2015, at 9:03 PM, Jordan Liggitt notifications@github.com wrote:
In pkg/build/api/v1beta1/types.go:
// the base64 encoded credentials. Supported auth methods are: ssh-privatekey.
- SourceSecretName string
json:"sourceSecretName,omitempty" description:"supported auth methods are: ssh-privatekey- SourceSecret *kapi.LocalObjectReference
json:"sourceSecret,omitempty" description:"supported auth methods are: ssh-privatekey"
I didn't think we could make changes like this... shouldn't this stay a string and be changed to a LocalObjectReference in conversion?—
Reply to this email directly or view it on GitHub.
There was a problem hiding this comment.
ok, didn't know how long it had been there
mfojtik
force-pushed
the
secrets-references
branch
from
7058667 to
edc50df
Compare
|
@liggitt validation updated with Prefix(), thanks! |
|
@deads2k to validate the secret, the Secret has to exist, right? What if I use PullSecret to reference a Secret that does not exists (yet)? |
I'm not talking about validating during the creation. I'd like to see the check done at the point of usage. Otherwise, you're mounting a secret that won't work. |
|
@deads2k gotcha |
mfojtik
force-pushed
the
secrets-references
branch
from
edc50df to
659c7a3
Compare
mfojtik
force-pushed
the
secrets-references
branch
from
659c7a3 to
a35be73
Compare
|
@deads2k the last issue that remains open here is validation of the Secret content and type that we agreed to no do during this sprint (or do it as follow-up). PTAL |
mfojtik
force-pushed
the
secrets-references
branch
2 times, most recently
from
7d32011 to
38b90de
Compare
|
I can't reproduce the Jenkins failure locally: π ~/go/src/github.com/openshift/origin ./hack/test-go.sh pkg/build/api/v1
ok github.com/openshift/origin/pkg/build/api/v1 0.009s |
mfojtik
force-pushed
the
secrets-references
branch
from
38b90de to
35aa9d9
Compare
|
@deads2k fixed conversions, this should be now green a good to go. |
|
Trello card for validation: https://trello.com/c/9nCfbdn4/583-validate-secrets-in-buildconfig-future-build [merge] |
|
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_requests_origin/2046/) (Image: devenv-fedora_1591) |
|
Evaluated for origin up to 35aa9d9 |
Merged by openshift-bot
No description provided.