Update CertificateSigningRequest apiversion to V1 #588

Kartik494 · 2021-09-09T02:10:48Z

Fixes #587

Updated `CertificateSigningRequest apiversion` to `V1`

Kartik494 · 2021-09-09T02:11:07Z

/release-note-none

Kartik494 · 2021-09-09T02:11:19Z

@xing-yang Could You please take a look

Kartik494 · 2021-09-09T02:17:36Z

When i update apiVersion to v1 ,i found we have to add signerName to it else it will fail.
After adding as suggested by in issue it works fine
ot-validation-service --secret snapshot-validation-secret --namespace default
creating certs in tmpdir /tmp/tmp.YauekRshRw
Generating RSA private key, 2048 bit long modulus (2 primes)
...............+++++
.........................................+++++
e is 65537 (0x010001)
certificatesigningrequest.certificates.k8s.io/snapshot-validation-service.default created
NAME AGE SIGNERNAME REQUESTOR REQUESTEDDURATION CONDITION
snapshot-validation-service.default 0s kubernetes.io/kube-apiserver-client minikube-user Pending
certificatesigningrequest.certificates.k8s.io/snapshot-validation-service.default approved
secret/snapshot-validation-secret created

Here is the outputs for pods and deployments
kubectl get pods -n cr
NAME READY STATUS RESTARTS AGE
snapshot-validation-deployment-6fdf6d59ff-5k4xx 1/1 Running 0 19m
snapshot-validation-deployment-6fdf6d59ff-5xl4q 1/1 Running 0 19m
snapshot-validation-deployment-6fdf6d59ff-clsls 1/1 Running 0 19m

kubectl get deployment -n cr
NAME READY UP-TO-DATE AVAILABLE AGE
snapshot-validation-deployment 3/3 3 3 19m

xing-yang

Can you run tests with invalid snapshots and verify that they are rejected by the webhook?

xing-yang · 2021-09-17T15:22:25Z

deploy/kubernetes/webhook-example/create-cert.sh

 kind: CertificateSigningRequest
 metadata:
-  name: ${csrName}
+ name: ${csrName}


It's better to indent 2 spaces rather than 1. Can you change this back?

Can you change this back? There's no need to make this change.

xing-yang · 2021-09-17T15:23:00Z

deploy/kubernetes/webhook-example/create-cert.sh

-  - digital signature
-  - key encipherment
-  - server auth
+ groups:


It's better to indent 2 spaces rather than 1. Can you change this back?

Sure i will addressed this changes

Kartik494 · 2021-09-28T09:59:19Z

@xing-yang i have addressed the changes regarding space indentation. I have validate the invalid snapshot v1 yaml for testing with webhook server as described in README and got the output as
kubectl create -f ./examples/kubernetes/invalid-snapshot-v1.yaml error: unable to recognize "./examples/kubernetes/invalid-snapshot-v1.yaml": no matches for kind "VolumeSnapshot" in version "snapshot.storage.k8s.io/v1"
Please let me know if this is right way to check the script or any other test i have to run.
Thanks!

xing-yang · 2021-10-14T15:58:55Z

@lajiao117 Does this PR fix your problem?

lajiao117 · 2021-10-15T01:18:51Z

@lajiao117 Does this PR fix your problem?

@xing-yang didn't, i used the signerName: kubernetes.io/kube-apiserver-client ,the webhook logs this err:
http: TLS handshake error from 10.244.0.0:30789: remote error: tls: bad certificate

k8s version is:v1.21.2

Kartik494 · 2021-10-22T06:35:41Z

@lajiao117 @xing-yang i have checked this in my environment and it's running fine. @lajiao117 if it's possible could you try this on another k8s cluster to recheck it. i have used minikube and kubeadm for respective versions

lajiao117 · 2021-10-22T07:42:53Z

@Kartik494 i recheck another k8s cluster ,it's the same err

i use the v1beta1 it works well

lajiao117 · 2021-10-22T07:44:40Z

@Kartik494 i used the webhook: https://github.com/kubernetes-csi/external-snapshotter/tree/v3.0.3
the csr yaml:

cat <<EOF | kubectl create -f -
apiVersion: certificates.k8s.io/v1
kind: CertificateSigningRequest
metadata:
name: ${csrName}
spec:
groups:

system:authenticated
signerName: kubernetes.io/kube-apiserver-client
request: $(cat ${tmpdir}/server.csr | base64 | tr -d '\n')
usages:
digital signature
key encipherment
client auth
EOF

Kartik494 · 2021-10-22T07:49:38Z

SO this error occur in latest release also i.e 4.2.1?

lajiao117 · 2021-10-22T08:17:59Z

SO this error occur in latest release also i.e 4.2.1?

@Kartik494 i just try 4.2.1,it logs the same err

Kartik494 · 2021-10-22T09:07:51Z

@lajiao117 Thanks for the update i got the output as
kubectl logs snapshot-validation-deployment-6fdf6d59ff-2rxcz I1022 09:04:58.308948 1 certwatcher.go:129] Updated current TLS certificate Starting webhook server I1022 09:04:58.309270 1 webhook.go:182] Starting certificate watcher

Kartik494 · 2021-10-22T09:11:23Z

i have doubt in the script file that is it ok to change usage attribute to client from server? my CSR got stuck in pending state? @xing-yang @lajiao117 any idea on this?.Please suggest

Kartik494 · 2021-10-22T09:29:20Z

Please refer this documentation here there is a specific term for use as client in authorization i.e client auth

lajiao117 · 2021-10-22T09:52:52Z

Please refer this documentation here there is a specific term for use as client in authorization i.e client auth

@Kartik494 i readed the doc, the script is no diffirent from the doc,

humblec · 2021-11-03T07:04:34Z

deploy/kubernetes/webhook-example/create-cert.sh

  usages:
  - digital signature
  - key encipherment
-  - server auth
+  - client auth


Why do we change or remove the server auth ?

HI @humblec i have not any exact idea of this, i am following certificate signing request template as describe in docs . So i have to keep it as server authorisation here? Any help on this would be appreciated. Thanks

lajiao117 · 2021-11-04T10:54:36Z

deploy/kubernetes/webhook-example/create-cert.sh

 spec:
  groups:
  - system:authenticated
  request: $(cat ${tmpdir}/server.csr | base64 | tr -d '\n')
+  signerName: kubernetes.io/kube-apiserver-client


signerName: kubernetes.io/kubelet-serving

lajiao117

use v1 version, signerName: kubernetes.io/kubelet-serving the organizations must be ["system:nodes"

openssl req -new -key ${tmpdir}/server-key.pem -subj "/CN=system:node:${service}.${namespace}.svc;/O=system:nodes" -out ${tmpdir}/server.csr -config ${tmpdir}/csr.conf

cat <<EOF | kubectl create -f -
apiVersion: certificates.k8s.io/v1
kind: CertificateSigningRequest
metadata:
name: ${csrName}
spec:
groups:

system:authenticated
request: $(cat ${tmpdir}/server.csr | base64 | tr -d '\n')
signerName: kubernetes.io/kubelet-serving
usages:
digital signature
key encipherment
server auth
EOF

lajiao117 · 2021-11-04T11:02:13Z

use v1 version, signerName: kubernetes.io/kubelet-serving the organizations must be ["system:nodes"

openssl req -new -key ${tmpdir}/server-key.pem -subj "/CN=system:node:${service}.${namespace}.svc;/O=system:nodes" -out ${tmpdir}/server.csr -config ${tmpdir}/csr.conf

cat <<EOF | kubectl create -f - apiVersion: certificates.k8s.io/v1 kind: CertificateSigningRequest metadata: name: ${csrName} spec: groups:

system:authenticated
request: $(cat ${tmpdir}/server.csr | base64 | tr -d '\n')
signerName: kubernetes.io/kubelet-serving
usages:

digital signature

key encipherment

server auth
EOF

@Kartik494 this script is ok for v1 version

Kartik494 · 2021-11-08T05:13:45Z

@lajiao117 Thanks for helping me out for this script ,it's working fine in my environment too.I have added a commit for the same.

Kartik494 · 2021-11-08T05:14:23Z

@xing-yang @humblec Could you please take a look at this updated commit . Thanks

xing-yang · 2021-11-17T21:30:27Z

/lgtm
/approve

k8s-ci-robot · 2021-11-17T21:30:39Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Kartik494, xing-yang

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [xing-yang]

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

xing-yang · 2021-11-17T21:31:17Z

@Kartik494 after this PR is merged, can you backport to release-5.0 branch?

Kartik494 · 2021-11-18T06:07:08Z

@xing-yang sure i will raise a PR for this.

Update CertificateSigningRequest apiversion to V1

xing-yang · 2021-11-18T18:48:59Z

I think this actually needs a release note.

Merge pull request #588 from Kartik494/ValidateWebhook

k8s-ci-robot added the do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. label Sep 9, 2021

k8s-ci-robot requested review from humblec and jingxu97 September 9, 2021 02:10

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Sep 9, 2021

k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Sep 9, 2021

xing-yang self-assigned this Sep 17, 2021

xing-yang reviewed Sep 17, 2021

View reviewed changes

xing-yang mentioned this pull request Sep 17, 2021

missing required field "signerName #587

Closed

Kartik494 force-pushed the ValidateWebhook branch from 3856875 to cf09bf8 Compare September 28, 2021 09:56

k8s-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Sep 28, 2021

humblec reviewed Nov 3, 2021

View reviewed changes

lajiao117 reviewed Nov 4, 2021

View reviewed changes

Kartik494 force-pushed the ValidateWebhook branch from cf09bf8 to a74c6c2 Compare November 8, 2021 05:11

Update CertificateSigningRequest apiversion

f8992bb

Kartik494 force-pushed the ValidateWebhook branch from a74c6c2 to f8992bb Compare November 12, 2021 04:37

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 17, 2021

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 17, 2021

k8s-ci-robot merged commit 4a9a465 into kubernetes-csi:master Nov 17, 2021

Kartik494 pushed a commit to Kartik494/external-snapshotter that referenced this pull request Nov 18, 2021

Merge pull request kubernetes-csi#588 from Kartik494/ValidateWebhook

c6da466

Update CertificateSigningRequest apiversion to V1

Kartik494 mentioned this pull request Nov 18, 2021

Merge pull request #588 from Kartik494/ValidateWebhook #613

Merged

k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesn't merit a release note. labels Nov 29, 2021

k8s-ci-robot added a commit that referenced this pull request Nov 29, 2021

Merge pull request #613 from Kartik494/backport-588

7cf8fea

Merge pull request #588 from Kartik494/ValidateWebhook

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update CertificateSigningRequest apiversion to V1 #588

Update CertificateSigningRequest apiversion to V1 #588

Kartik494 commented Sep 9, 2021 •

edited by xing-yang

Loading

Kartik494 commented Sep 9, 2021

Kartik494 commented Sep 9, 2021

Kartik494 commented Sep 9, 2021

xing-yang left a comment

xing-yang Sep 17, 2021

xing-yang Nov 11, 2021

Kartik494 Nov 12, 2021

xing-yang Sep 17, 2021

Kartik494 Sep 22, 2021

Kartik494 commented Sep 28, 2021

xing-yang commented Oct 14, 2021

lajiao117 commented Oct 15, 2021 •

edited

Loading

Kartik494 commented Oct 22, 2021

lajiao117 commented Oct 22, 2021 •

edited

Loading

lajiao117 commented Oct 22, 2021 •

edited

Loading

Kartik494 commented Oct 22, 2021

lajiao117 commented Oct 22, 2021

Kartik494 commented Oct 22, 2021

Kartik494 commented Oct 22, 2021

Kartik494 commented Oct 22, 2021

lajiao117 commented Oct 22, 2021 •

edited

Loading

humblec Nov 3, 2021

Kartik494 Nov 3, 2021

lajiao117 Nov 4, 2021

lajiao117 left a comment

lajiao117 commented Nov 4, 2021

Kartik494 commented Nov 8, 2021

Kartik494 commented Nov 8, 2021

xing-yang commented Nov 17, 2021

k8s-ci-robot commented Nov 17, 2021

xing-yang commented Nov 17, 2021

Kartik494 commented Nov 18, 2021

xing-yang commented Nov 18, 2021

Update CertificateSigningRequest apiversion to V1 #588

Update CertificateSigningRequest apiversion to V1 #588

Conversation

Kartik494 commented Sep 9, 2021 • edited by xing-yang Loading

Kartik494 commented Sep 9, 2021

Kartik494 commented Sep 9, 2021

Kartik494 commented Sep 9, 2021

xing-yang left a 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Kartik494 commented Sep 28, 2021

xing-yang commented Oct 14, 2021

lajiao117 commented Oct 15, 2021 • edited Loading

Kartik494 commented Oct 22, 2021

lajiao117 commented Oct 22, 2021 • edited Loading

lajiao117 commented Oct 22, 2021 • edited Loading

Kartik494 commented Oct 22, 2021

lajiao117 commented Oct 22, 2021

Kartik494 commented Oct 22, 2021

Kartik494 commented Oct 22, 2021

Kartik494 commented Oct 22, 2021

lajiao117 commented Oct 22, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lajiao117 left a comment

Choose a reason for hiding this comment

lajiao117 commented Nov 4, 2021

Kartik494 commented Nov 8, 2021

Kartik494 commented Nov 8, 2021

xing-yang commented Nov 17, 2021

k8s-ci-robot commented Nov 17, 2021

xing-yang commented Nov 17, 2021

Kartik494 commented Nov 18, 2021

xing-yang commented Nov 18, 2021

Kartik494 commented Sep 9, 2021 •

edited by xing-yang

Loading

lajiao117 commented Oct 15, 2021 •

edited

Loading

lajiao117 commented Oct 22, 2021 •

edited

Loading

lajiao117 commented Oct 22, 2021 •

edited

Loading

lajiao117 commented Oct 22, 2021 •

edited

Loading