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

Apply resource transformers #14836

Merged

Conversation

php-coder
Copy link
Contributor

@php-coder php-coder commented Jun 22, 2017

A follow-up to #14798

BuildStorageFactory from vendor/k8s.io/kubernetes/cmd/kube-apiserver/app/server.go doesn't get executed on OpenShift and hence encryption config wasn't being read and the transformers weren't being applied. This PR fixes this.

PTAL @smarterclayton @liggitt @simo5

@@ -216,6 +217,17 @@ func BuildStorageFactory(masterConfig configapi.MasterConfig, server *kapiserver
// keep Deployments in extensions for backwards compatibility, we'll have to migrate at some point, eventually
storageFactory.AddCohabitatingResources(extensions.Resource("deployments"), apps.Resource("deployments"))

if server.Etcd.EncryptionProviderConfigFilepath != "" {
glog.V(4).Infof("Reading encryption configuration from %q", server.Etcd.EncryptionProviderConfigFilepath)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This log line was added by me. I think it won't hurt but it simplifies a debugging process.

@smarterclayton
Copy link
Contributor

smarterclayton commented Jun 22, 2017 via email

@php-coder
Copy link
Contributor Author

@deads2k
Copy link
Contributor

deads2k commented Jun 22, 2017

This is more of a vision question. If we're going to change our deployment model to mirror kube with separate processes and we want our configs to look consistent, then this change is ok. If we want to keep our single process with a nicer config, then this seems odd since we'd want to auto-create the keys to encrypt by default.

@smarterclayton
Copy link
Contributor

smarterclayton commented Jun 22, 2017 via email

@php-coder
Copy link
Contributor Author

@enj
Copy link
Contributor

enj commented Jun 24, 2017

@openshift/security

@php-coder
Copy link
Contributor Author

2 weeks and no objections... I think it can be merged then :)

@smarterclayton
Copy link
Contributor

[test]

@smarterclayton smarterclayton added this to the 3.6.0 milestone Jul 7, 2017
@php-coder
Copy link
Contributor Author

php-coder commented Jul 7, 2017

test flake #9309

@php-coder
Copy link
Contributor Author

[test]

@openshift-bot
Copy link
Contributor

Evaluated for origin test up to a53531e

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_request_origin/3035/) (Base Commit: fe7173b) (PR Branch Commit: a53531e)

@smarterclayton
Copy link
Contributor

[merge][severity:blocker]

@php-coder
Copy link
Contributor Author

Test flake #14897

@0xmichalis
Copy link
Contributor

#14897 [merge][severity:blocker]

@openshift-bot
Copy link
Contributor

Evaluated for origin merge up to a53531e

@openshift-bot
Copy link
Contributor

openshift-bot commented Jul 11, 2017

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_request_origin/1260/) (Base Commit: d4f473a) (PR Branch Commit: a53531e) (Extended Tests: blocker) (Image: devenv-rhel7_6433)

@openshift-bot openshift-bot merged commit 6c5dafb into openshift:master Jul 11, 2017
@php-coder php-coder deleted the set_encryption_transformers branch July 11, 2017 12:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants