Inject kube-apiserver pods trust stores with trusted ca bundle by stlaz · Pull Request #552 · openshift/cluster-kube-apiserver-operator

stlaz · 2019-08-19T15:04:58Z

Adds the trusted store injection and a watchdog that will restart
the kube-apiserver process on the trusted CA bundle change.

stlaz · 2019-08-20T07:01:52Z

/retest

stlaz · 2019-08-20T12:53:01Z

In order for this to work correctly, we'll have to add the injected cm somewhere into https://github.com/openshift/cluster-kube-apiserver-operator/blob/master/pkg/operator/starter.go#L180.

We'll need to get a CM with the proper key there somewhere, possibly adding a CM apply from the targetconfigcontroller that'll read it from the CM that's being injected 🤔

bindata/v4.1.0/kube-apiserver/pod.yaml

mfojtik · 2019-08-21T13:42:17Z

This need e2e test.

mfojtik · 2019-08-22T08:23:06Z

You can't wire file watchdog to kube api server pod, because when the configmap change, the watchdog will deliberately kill all 3 api servers immediately (to restart them).

I think we need to roll new revision when the config map change. To do that you need to add the configmap with proxy CA into RevisionConfigMaps and then create optional volume mount in kube apiserver pod.

@sttts @deads2k correct?

stlaz · 2019-08-22T09:55:14Z

@mfojtik: seeing the issue I was mentioning earlier that prevents kube-apiserver pods from running:

'MountVolume.SetUp failed for volume "trusted-ca-bundle" : configmaps "trusted-ca-bundle-2"
    is forbidden: User "system:node:ip-10-0-149-86.ec2.internal" cannot get resource
    "configmaps" in API group "" in the namespace "openshift-kube-apiserver": no relationship
    found between node "ip-10-0-149-86.ec2.internal" and this object'

bindata/v4.1.0/kube-apiserver/pod.yaml

mfojtik · 2019-08-22T11:54:17Z

pkg/operator/starter.go

 	{Name: "sa-token-signing-certs"},
+
+	// this is a copy of trusted-ca-bundle CM but with key modified to "tls-ca-bundle.pem" so that we can mount it the way we need
+	{Name: "trusted-ca-bundle"},


I think you need this in CertConfigMaps /cc @deads2k

I guess you're right, in the logs, I can see that the dirs created are:

"/etc/kubernetes/static-pod-resources/kube-apiserver-certs/configmaps/client-ca"

but on the other hand also

"/etc/kubernetes/static-pod-resources/kube-apiserver-pod-2/configmaps/trusted-ca-bundle"

which would explain why the Directory hostMount assumption fails, I'll move this to the CertConfigMaps then.

mfojtik · 2019-08-22T11:55:05Z

bindata/v4.1.0/kube-apiserver/pod.yaml

      name: cert-dir
    - mountPath: /var/log/kube-apiserver
      name: audit-dir
+    - mountPath: /etc/pki/ca-trust/extracted/pem/


is this overriding existing directory? in case there is no trusted-ca-bundle config map, we erase that dir?

(i wonder if this should be handled in the installer pod controller)

That's a good point, I think the hostPath directory will only appear once the appropriate CM exists. I guess setting type: Directory for the matching volume might solve the problem of the hostPath directory not existing.

@stlaz the mount should be optional.

@danehans I don't think there is a way how that could work here

Nvm, we found a way around it to make it optional

pkg/operator/targetconfigcontroller/targetconfigcontroller.go

stlaz · 2019-08-22T13:06:02Z

/test e2e-aws
Looks like installer bug:

level=error msg="Error: Provider produced inconsistent result after apply"
level=error
level=error msg="When applying changes to module.bootstrap.aws_s3_bucket.ignition, provider"
level=error msg="\"aws\" produced an unexpected new value for was present, but now absent."
level=error
level=error msg="This is a bug in the provider, which should be reported in the provider's own"
level=error msg="issue tracker."

@wking

wking · 2019-08-22T13:23:30Z

Shared upstream.

stlaz · 2019-08-22T14:00:01Z

/hold
I was suggested to use bash wrapper to wait for the file mount location to exist and then just copy to proper path, probably a better solution

stlaz · 2019-08-22T15:02:57Z

The dir check in hostPath seemed to have failed, I wonder if removing revision from the path will help

stlaz · 2019-08-22T15:42:57Z

/retest

stlaz · 2019-08-22T19:59:49Z

Pods are still getting stuck in unitialized on the mount, it would appear the directory just does not appear...

stlaz · 2019-08-23T17:20:47Z

/retest
The failures don’t seem to be related to this PR

stlaz · 2019-08-24T04:45:12Z

/retest

Adds the trusted store injection and a watchdog that will restart the kube-apiserver process on the trusted CA bundle change.

mfojtik · 2019-08-26T09:45:34Z

pkg/operator/targetconfigcontroller/targetconfigcontroller.go

 	return resourceapply.ApplyConfigMap(client, recorder, requiredConfigMap)
 }

+func ensureKubeAPIServerTrustedCA(client coreclientv1.CoreV1Interface, recorder events.Recorder) error {


I do believe these should have comments describing their purpose.

mfojtik · 2019-08-26T09:48:39Z

/lgtm

I don't like the bash in the pod spec, but for can't think of simpler solution for now (we can improve this in future).

openshift-ci-robot · 2019-08-26T09:49:04Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mfojtik, stlaz

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [mfojtik]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-bot · 2019-08-26T10:46:34Z

/retest

Please review the full test history for this PR and help us cut down flakes.

cuppett · 2019-08-26T14:50:37Z

/test e2e-aws-upgrade

openshift-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Aug 19, 2019

openshift-ci-robot requested review from mfojtik and sttts August 19, 2019 15:05

stlaz force-pushed the add_trusted_ca_bundle branch from 16308a2 to e65e1a5 Compare August 20, 2019 12:48

stlaz force-pushed the add_trusted_ca_bundle branch from e65e1a5 to 75b7a0c Compare August 21, 2019 08:57

openshift-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Aug 21, 2019

stlaz force-pushed the add_trusted_ca_bundle branch 2 times, most recently from 3f78b39 to bb80690 Compare August 21, 2019 11:00

mfojtik reviewed Aug 21, 2019

View reviewed changes

bindata/v4.1.0/kube-apiserver/pod.yaml Outdated Show resolved Hide resolved

stlaz force-pushed the add_trusted_ca_bundle branch from bb80690 to 1cccaa2 Compare August 21, 2019 13:47

stlaz force-pushed the add_trusted_ca_bundle branch from 1cccaa2 to eeadf44 Compare August 22, 2019 08:31

openshift-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Aug 22, 2019

stlaz force-pushed the add_trusted_ca_bundle branch from eeadf44 to 9a4c590 Compare August 22, 2019 10:50

mfojtik reviewed Aug 22, 2019

View reviewed changes

bindata/v4.1.0/kube-apiserver/pod.yaml Outdated Show resolved Hide resolved

mfojtik reviewed Aug 22, 2019

View reviewed changes

pkg/operator/targetconfigcontroller/targetconfigcontroller.go Outdated Show resolved Hide resolved

mfojtik reviewed Aug 22, 2019

View reviewed changes

pkg/operator/targetconfigcontroller/targetconfigcontroller.go Outdated Show resolved Hide resolved

stlaz force-pushed the add_trusted_ca_bundle branch from 9a4c590 to f8bac6d Compare August 22, 2019 12:49

stlaz force-pushed the add_trusted_ca_bundle branch from f8bac6d to c104bee Compare August 22, 2019 15:02

stlaz force-pushed the add_trusted_ca_bundle branch from c104bee to 8dba95a Compare August 22, 2019 18:33

stlaz force-pushed the add_trusted_ca_bundle branch 2 times, most recently from 3087341 to 5c67756 Compare August 23, 2019 07:23

stlaz changed the title ~~WIP: Inject kube-apiserver pods trust stores with trusted ca bundle~~ Inject kube-apiserver pods trust stores with trusted ca bundle Aug 23, 2019

openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 23, 2019

stlaz force-pushed the add_trusted_ca_bundle branch 6 times, most recently from ce6cd93 to 86b7dc9 Compare August 23, 2019 14:36

sttts force-pushed the add_trusted_ca_bundle branch from ce683a2 to 2b4ad94 Compare August 26, 2019 09:42

Inject kube-apiserver pods trust stores with trusted ca bundle

afc2bbe

Adds the trusted store injection and a watchdog that will restart the kube-apiserver process on the trusted CA bundle change.

sttts force-pushed the add_trusted_ca_bundle branch from 2b4ad94 to afc2bbe Compare August 26, 2019 09:42

mfojtik reviewed Aug 26, 2019

View reviewed changes

openshift-ci-robot assigned mfojtik Aug 26, 2019

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Aug 26, 2019

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 26, 2019

openshift-merge-robot merged commit 189121a into openshift:master Aug 26, 2019

soltysh mentioned this pull request Dec 12, 2019

Bug 1772756: Inject kube-controller-manager pods trust stores with trusted ca bundle openshift/cluster-kube-controller-manager-operator#325

Merged

Conversation

stlaz commented Aug 19, 2019

Uh oh!

stlaz commented Aug 20, 2019

Uh oh!

stlaz commented Aug 20, 2019

Uh oh!

Uh oh!

mfojtik commented Aug 21, 2019

Uh oh!

mfojtik commented Aug 22, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stlaz commented Aug 22, 2019

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

stlaz commented Aug 22, 2019

Uh oh!

wking commented Aug 22, 2019

Uh oh!

stlaz commented Aug 22, 2019

Uh oh!

stlaz commented Aug 22, 2019

Uh oh!

stlaz commented Aug 22, 2019

Uh oh!

stlaz commented Aug 22, 2019

Uh oh!

stlaz commented Aug 23, 2019

Uh oh!

stlaz commented Aug 24, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mfojtik commented Aug 26, 2019

Uh oh!

openshift-ci-robot commented Aug 26, 2019

Uh oh!

openshift-bot commented Aug 26, 2019

Uh oh!

cuppett commented Aug 26, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

mfojtik commented Aug 22, 2019 •

edited

Loading