Support ExtendedSecret certs signed by the Kube API Server #484

edwardstudy · 2019-08-07T09:46:28Z

Added two options to support CSR for ExtendedSecret:

SignerType       SignerType        `json:"signerType,omitempty"`
Usages           []certv1.KeyUsage `json:"usages,omitempty"`

Added CSR controller to watch CSR and approve it and then persists certificate signed by Kube API Server

Verification steps:

$ k apply -f ./docs/examples/extended-secret/certificate.yaml

$ k get secret
NAME                                   TYPE                                  DATA   AGE
...
nats-deployment.var-nats-cert    # This cert signed by API Server

cfdreddbot · 2019-08-07T09:46:29Z

✅ Hey edwardstudy! The commit authors and yourself have already signed the CLA.

pkg/kube/apis/extendedsecret/v1alpha1/types.go

pkg/credsgen/in_memory_generator/certificate.go

aduffeck · 2019-08-08T13:14:10Z

pkg/kube/controllers/extendedsecret/certificatesigningrequest_reconciler.go

+		return reconcile.Result{}, nil
+	}
+
+	err = r.approveRequest(ctx, instance.Name)


I think this should better go into an else branch.

And couldn't we pass the object directly instead of its name to get rid of another get request?

Oh, it's helpful for reducing update conflicts.
And r.client could not update subresource approval. we have to use client-go.client to update CSR approval directly:
kubernetes-sigs/controller-runtime#172

aduffeck · 2019-08-08T13:16:51Z

pkg/kube/util/names/names.go

@@ -180,6 +180,11 @@ func OrdinalFromPodName(name string) int {
 	return podOrdinal
 }

+// PrivateKeySecretName returns a Secret name for a given ExtendedSecret
+func PrivateKeySecretName(extendedSecretName string) string {


Could we make this name a bit more precise, i.e. CsrPrivateKeySecretName?

aduffeck · 2019-08-08T13:26:32Z

pkg/kube/controllers/extendedsecret/certificatesigningrequest_reconciler.go

+	}
+
+	if len(instance.Status.Certificate) != 0 {
+		annotations := instance.GetAnnotations()


I'm just wondering, why do we need these annotations? Couldn't we get the secret name and namespace from the owner object (ExtendedSecret)?

The truth is that:

CSR is no-namespaced

the owner and owned object are both namespace-scoped and assumed in the same namespace

kubernetes-sigs/controller-runtime#214 (comment)

So getting the owner's namespace from CSR could not get right namespace.
My idea is to set the owner's namespace to annotations.

pkg/kube/controllers/extendedsecret/certificatesigningrequest_reconciler.go

aduffeck · 2019-08-08T13:46:55Z

pkg/kube/controllers/extendedsecret/extendedsecret_reconciler.go

@@ -124,6 +127,7 @@ func (r *ReconcileExtendedSecret) Reconcile(request reconcile.Request) (reconcil
 func (r *ReconcileExtendedSecret) updateExSecret(ctx context.Context, instance *esv1.ExtendedSecret) error {
 	obj := instance.DeepCopy()
 	op, err := controllerutil.CreateOrUpdate(ctx, r.client, obj, func() error {
+		obj.Spec = instance.Spec


Why is this change needed?

It's helpful to update default spec.request.signerType if it's certificate.

refactored CreateOrUpdate mutateFn. As I said, empty spec.request.signerType would be populated as local.

viovanov · 2019-08-14T05:30:18Z

docs/examples/extended-secret/certificate.yaml

+        - '*.foo.com'
+      commonName: routerSSL
+      isCA: false
+      signerType: external


This looks like it's not using the API server to sign

Removed CA infos. Sorry for misleading example.

docs/controllers/extendedsecret.md

pkg/credsgen/in_memory_generator/certificate.go

pkg/kube/apis/extendedsecret/v1alpha1/types.go

edwardstudy · 2019-08-16T08:14:39Z

cf-operator can consume following CR and create cert signed by API server now:

---
apiVersion: fissile.cloudfoundry.org/v1alpha1
kind: ExtendedSecret
metadata:
  name: generate-certificate
spec:
  request:
    certificate:
      alternativeNames:
        - foo.com
        - '*.foo.com'
      commonName: routerSSL
      signerType: cluster
      usages: ["server auth", "client auth"]
  secretName: gen-certificate
  type: certificate

aduffeck

lgtm besides the test expectation change

e2e/kube/examples_test.go

edwardstudy added the in progress label Aug 7, 2019

edwardstudy force-pushed the ed/csr branch from cfd2eb6 to 3d6bdb9 Compare August 8, 2019 07:59

edwardstudy removed the in progress label Aug 8, 2019

edwardstudy force-pushed the ed/csr branch 4 times, most recently from c19b097 to 89f93ac Compare August 8, 2019 09:21

aduffeck reviewed Aug 8, 2019

View reviewed changes

edwardstudy force-pushed the ed/csr branch 3 times, most recently from 428648d to d0a3e92 Compare August 13, 2019 09:11

viovanov suggested changes Aug 14, 2019

View reviewed changes

edwardstudy force-pushed the ed/csr branch from d06aff4 to 50e9f48 Compare August 14, 2019 09:13

edwardstudy requested a review from viovanov August 14, 2019 09:27

edwardstudy force-pushed the ed/csr branch 10 times, most recently from 281839a to fdada63 Compare August 16, 2019 02:44

edwardstudy added 5 commits August 16, 2019 15:59

Create CertificateSigningRequest object

95fc8b5

Add CSR controller to approve cert request and create relative secret

deb09b6

Add cert cases and docs

a473575

Wrap error messge

d69f44e

Clean up CA settings in cluster signed cert and update docs

3627c3b

edwardstudy force-pushed the ed/csr branch from fdada63 to 3627c3b Compare August 16, 2019 08:06

aduffeck suggested changes Aug 16, 2019

View reviewed changes

e2e/kube/examples_test.go Show resolved Hide resolved

Merge branch 'master' into ed/csr

6efe1f5

aduffeck approved these changes Aug 19, 2019

View reviewed changes

viovanov approved these changes Aug 19, 2019

View reviewed changes

aduffeck merged commit e18fa78 into master Aug 19, 2019

aduffeck deleted the ed/csr branch August 19, 2019 08:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support ExtendedSecret certs signed by the Kube API Server #484

Support ExtendedSecret certs signed by the Kube API Server #484

edwardstudy commented Aug 7, 2019 •

edited

Loading

cfdreddbot commented Aug 7, 2019

aduffeck Aug 8, 2019

aduffeck Aug 8, 2019

edwardstudy Aug 9, 2019

aduffeck Aug 8, 2019

aduffeck Aug 8, 2019

edwardstudy Aug 8, 2019 •

edited

Loading

aduffeck Aug 8, 2019

edwardstudy Aug 9, 2019

edwardstudy Aug 16, 2019 •

edited

Loading

viovanov Aug 14, 2019

edwardstudy Aug 14, 2019

edwardstudy commented Aug 16, 2019 •

edited

Loading

aduffeck left a comment

Support ExtendedSecret certs signed by the Kube API Server #484

Support ExtendedSecret certs signed by the Kube API Server #484

Conversation

edwardstudy commented Aug 7, 2019 • edited Loading

cfdreddbot commented Aug 7, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

edwardstudy Aug 8, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

edwardstudy Aug 16, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

edwardstudy commented Aug 16, 2019 • edited Loading

aduffeck left a comment

Choose a reason for hiding this comment

edwardstudy commented Aug 7, 2019 •

edited

Loading

edwardstudy Aug 8, 2019 •

edited

Loading

edwardstudy Aug 16, 2019 •

edited

Loading

edwardstudy commented Aug 16, 2019 •

edited

Loading