From e5d52f237433c467f62d0314e7feef58b54f20b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arda=20G=C3=BC=C3=A7l=C3=BC?= Date: Wed, 24 Aug 2022 09:17:05 +0300 Subject: [PATCH] (kubectl certificates): Remove certificates/v1beta1 client usage certificates/v1beta1 was deprecated in 1.19 and we can safely clear it's dependencies in `kubectl certificates` command. Kubernetes-commit: 48b8ee0d3ba40a19d40a7404f890031fe22bc111 --- pkg/cmd/certificates/certificates.go | 61 ++++------------------- pkg/cmd/certificates/certificates_test.go | 61 +---------------------- 2 files changed, 13 insertions(+), 109 deletions(-) diff --git a/pkg/cmd/certificates/certificates.go b/pkg/cmd/certificates/certificates.go index 12474f06..d0f39932 100644 --- a/pkg/cmd/certificates/certificates.go +++ b/pkg/cmd/certificates/certificates.go @@ -24,7 +24,6 @@ import ( "github.com/spf13/cobra" certificatesv1 "k8s.io/api/certificates/v1" - certificatesv1beta1 "k8s.io/api/certificates/v1beta1" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -34,7 +33,6 @@ import ( "k8s.io/cli-runtime/pkg/printers" "k8s.io/cli-runtime/pkg/resource" v1 "k8s.io/client-go/kubernetes/typed/certificates/v1" - "k8s.io/client-go/kubernetes/typed/certificates/v1beta1" cmdutil "k8s.io/kubectl/pkg/cmd/util" "k8s.io/kubectl/pkg/scheme" "k8s.io/kubectl/pkg/util/i18n" @@ -69,9 +67,8 @@ type CertificateOptions struct { csrNames []string outputStyle string - certificatesV1Client v1.CertificatesV1Interface - certificatesV1Beta1Client v1beta1.CertificatesV1beta1Interface - builder *resource.Builder + certificatesV1Client v1.CertificatesV1Interface + builder *resource.Builder genericclioptions.IOStreams } @@ -110,11 +107,6 @@ func (o *CertificateOptions) Complete(restClientGetter genericclioptions.RESTCli return err } - o.certificatesV1Beta1Client, err = v1beta1.NewForConfig(clientConfig) - if err != nil { - return err - } - return nil } @@ -242,14 +234,9 @@ func (o *CertificateOptions) modifyCertificateCondition(builder *resource.Builde } var csr runtime.Object // get a typed object - // first try v1 csr, err = o.certificatesV1Client.CertificateSigningRequests().Get(context.TODO(), obj.GetName(), metav1.GetOptions{}) if apierrors.IsNotFound(err) { - // fall back to v1beta1 - csr, err = o.certificatesV1Beta1Client.CertificateSigningRequests().Get(context.TODO(), obj.GetName(), metav1.GetOptions{}) - } - if apierrors.IsNotFound(err) { - return fmt.Errorf("could not find v1 or v1beta1 version of %s: %v", obj.GetName(), err) + return fmt.Errorf("could not find v1 version of %s: %v", obj.GetName(), err) } if err != nil { return err @@ -260,14 +247,12 @@ func (o *CertificateOptions) modifyCertificateCondition(builder *resource.Builde return err } if !hasCondition || force { - switch modifiedCSR := modifiedCSR.(type) { - case *certificatesv1.CertificateSigningRequest: - _, err = o.certificatesV1Client.CertificateSigningRequests().UpdateApproval(context.TODO(), modifiedCSR.Name, modifiedCSR, metav1.UpdateOptions{}) - case *certificatesv1beta1.CertificateSigningRequest: - _, err = o.certificatesV1Beta1Client.CertificateSigningRequests().UpdateApproval(context.TODO(), modifiedCSR, metav1.UpdateOptions{}) - default: - return fmt.Errorf("can only handle certificates.k8s.io CertificateSigningRequest objects, got %T", modifiedCSR) + if mCSR, ok := modifiedCSR.(*certificatesv1.CertificateSigningRequest); ok { + _, err = o.certificatesV1Client.CertificateSigningRequests().UpdateApproval(context.TODO(), mCSR.Name, mCSR, metav1.UpdateOptions{}) + } else { + return fmt.Errorf("can only handle certificates.k8s.io CertificateSigningRequest objects, got %T", mCSR) } + if apierrors.IsConflict(err) && i < 10 { if err := info.Get(); err != nil { return err @@ -291,9 +276,8 @@ func (o *CertificateOptions) modifyCertificateCondition(builder *resource.Builde } func addConditionIfNeeded(mustNotHaveConditionType, conditionType, reason, message string) func(runtime.Object) (runtime.Object, bool, error) { - return func(csr runtime.Object) (runtime.Object, bool, error) { - switch csr := csr.(type) { - case *certificatesv1.CertificateSigningRequest: + return func(obj runtime.Object) (runtime.Object, bool, error) { + if csr, ok := obj.(*certificatesv1.CertificateSigningRequest); ok { var alreadyHasCondition bool for _, c := range csr.Status.Conditions { if string(c.Type) == mustNotHaveConditionType { @@ -314,30 +298,7 @@ func addConditionIfNeeded(mustNotHaveConditionType, conditionType, reason, messa LastUpdateTime: metav1.Now(), }) return csr, false, nil - - case *certificatesv1beta1.CertificateSigningRequest: - var alreadyHasCondition bool - for _, c := range csr.Status.Conditions { - if string(c.Type) == mustNotHaveConditionType { - return nil, false, fmt.Errorf("certificate signing request %q is already %s", csr.Name, c.Type) - } - if string(c.Type) == conditionType { - alreadyHasCondition = true - } - } - if alreadyHasCondition { - return csr, true, nil - } - csr.Status.Conditions = append(csr.Status.Conditions, certificatesv1beta1.CertificateSigningRequestCondition{ - Type: certificatesv1beta1.RequestConditionType(conditionType), - Status: corev1.ConditionTrue, - Reason: reason, - Message: message, - LastUpdateTime: metav1.Now(), - }) - return csr, false, nil - - default: + } else { return csr, false, nil } } diff --git a/pkg/cmd/certificates/certificates_test.go b/pkg/cmd/certificates/certificates_test.go index bd0941c5..a901943d 100644 --- a/pkg/cmd/certificates/certificates_test.go +++ b/pkg/cmd/certificates/certificates_test.go @@ -27,7 +27,6 @@ import ( "github.com/spf13/cobra" certificatesv1 "k8s.io/api/certificates/v1" - certificatesv1beta1 "k8s.io/api/certificates/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/cli-runtime/pkg/genericclioptions" @@ -63,20 +62,7 @@ func TestCertificates(t *testing.T) { expectOutput: `approved`, }, { - name: "approve existing, no v1", - nov1: true, - command: "approve", - args: []string{"certificatesigningrequests.v1beta1.certificates.k8s.io/existing"}, - expectActions: []string{ - `GET /apis/certificates.k8s.io/v1beta1/certificatesigningrequests/existing`, // unstructured get - `GET /apis/certificates.k8s.io/v1/certificatesigningrequests/existing`, // typed get, 404 - `GET /apis/certificates.k8s.io/v1beta1/certificatesigningrequests/existing`, // typed get fallback - `PUT /apis/certificates.k8s.io/v1beta1/certificatesigningrequests/existing/approval`, - }, - expectOutput: `approved`, - }, - { - name: "approve existing, no v1 or v1beta1", + name: "approve existing, no v1", nov1: true, nov1beta1: true, command: "approve", @@ -133,20 +119,7 @@ func TestCertificates(t *testing.T) { expectOutput: `denied`, }, { - name: "deny existing, no v1", - nov1: true, - command: "deny", - args: []string{"certificatesigningrequests.v1beta1.certificates.k8s.io/existing"}, - expectActions: []string{ - `GET /apis/certificates.k8s.io/v1beta1/certificatesigningrequests/existing`, // unstructured get - `GET /apis/certificates.k8s.io/v1/certificatesigningrequests/existing`, // typed get, 404 - `GET /apis/certificates.k8s.io/v1beta1/certificatesigningrequests/existing`, // typed get fallback - `PUT /apis/certificates.k8s.io/v1beta1/certificatesigningrequests/existing/approval`, - }, - expectOutput: `denied`, - }, - { - name: "deny existing, no v1 or v1beta1", + name: "deny existing, no v1", nov1: true, nov1beta1: true, command: "deny", @@ -202,32 +175,18 @@ func TestCertificates(t *testing.T) { TypeMeta: metav1.TypeMeta{APIVersion: "certificates.k8s.io/v1", Kind: "CertificateSigningRequest"}, ObjectMeta: metav1.ObjectMeta{Name: "existing"}, } - existingV1beta1 := &certificatesv1beta1.CertificateSigningRequest{ - TypeMeta: metav1.TypeMeta{APIVersion: "certificates.k8s.io/v1beta1", Kind: "CertificateSigningRequest"}, - ObjectMeta: metav1.ObjectMeta{Name: "existing"}, - } approvedV1 := &certificatesv1.CertificateSigningRequest{ TypeMeta: metav1.TypeMeta{APIVersion: "certificates.k8s.io/v1", Kind: "CertificateSigningRequest"}, ObjectMeta: metav1.ObjectMeta{Name: "approved"}, Status: certificatesv1.CertificateSigningRequestStatus{Conditions: []certificatesv1.CertificateSigningRequestCondition{{Type: certificatesv1.CertificateApproved}}}, } - approvedV1beta1 := &certificatesv1beta1.CertificateSigningRequest{ - TypeMeta: metav1.TypeMeta{APIVersion: "certificates.k8s.io/v1beta1", Kind: "CertificateSigningRequest"}, - ObjectMeta: metav1.ObjectMeta{Name: "existing"}, - Status: certificatesv1beta1.CertificateSigningRequestStatus{Conditions: []certificatesv1beta1.CertificateSigningRequestCondition{{Type: certificatesv1beta1.CertificateApproved}}}, - } deniedV1 := &certificatesv1.CertificateSigningRequest{ TypeMeta: metav1.TypeMeta{APIVersion: "certificates.k8s.io/v1", Kind: "CertificateSigningRequest"}, ObjectMeta: metav1.ObjectMeta{Name: "denied"}, Status: certificatesv1.CertificateSigningRequestStatus{Conditions: []certificatesv1.CertificateSigningRequestCondition{{Type: certificatesv1.CertificateDenied}}}, } - deniedV1beta1 := &certificatesv1beta1.CertificateSigningRequest{ - TypeMeta: metav1.TypeMeta{APIVersion: "certificates.k8s.io/v1beta1", Kind: "CertificateSigningRequest"}, - ObjectMeta: metav1.ObjectMeta{Name: "denied"}, - Status: certificatesv1beta1.CertificateSigningRequestStatus{Conditions: []certificatesv1beta1.CertificateSigningRequestCondition{{Type: certificatesv1beta1.CertificateDenied}}}, - } actions := []string{} fakeClient := fake.CreateHTTPClient(func(req *http.Request) (*http.Response, error) { @@ -235,40 +194,24 @@ func TestCertificates(t *testing.T) { switch p, m := req.URL.Path, req.Method; { case tc.nov1 && strings.HasPrefix(p, "/apis/certificates.k8s.io/v1/"): return &http.Response{StatusCode: http.StatusNotFound, Body: ioutil.NopCloser(bytes.NewBuffer([]byte{}))}, nil - case tc.nov1beta1 && strings.HasPrefix(p, "/apis/certificates.k8s.io/v1beta1/"): - return &http.Response{StatusCode: http.StatusNotFound, Body: ioutil.NopCloser(bytes.NewBuffer([]byte{}))}, nil case p == "/apis/certificates.k8s.io/v1/certificatesigningrequests/missing" && m == http.MethodGet: return &http.Response{StatusCode: http.StatusNotFound}, nil - case p == "/apis/certificates.k8s.io/v1beta1/certificatesigningrequests/missing" && m == http.MethodGet: - return &http.Response{StatusCode: http.StatusNotFound}, nil case p == "/apis/certificates.k8s.io/v1/certificatesigningrequests/existing" && m == http.MethodGet: return &http.Response{StatusCode: http.StatusOK, Header: cmdtesting.DefaultHeader(), Body: cmdtesting.ObjBody(codec, existingV1)}, nil case p == "/apis/certificates.k8s.io/v1/certificatesigningrequests/existing/approval" && m == http.MethodPut: return &http.Response{StatusCode: http.StatusOK, Header: cmdtesting.DefaultHeader(), Body: cmdtesting.ObjBody(codec, existingV1)}, nil - case p == "/apis/certificates.k8s.io/v1beta1/certificatesigningrequests/existing" && m == http.MethodGet: - return &http.Response{StatusCode: http.StatusOK, Header: cmdtesting.DefaultHeader(), Body: cmdtesting.ObjBody(codec, existingV1beta1)}, nil - case p == "/apis/certificates.k8s.io/v1beta1/certificatesigningrequests/existing/approval" && m == http.MethodPut: - return &http.Response{StatusCode: http.StatusOK, Header: cmdtesting.DefaultHeader(), Body: cmdtesting.ObjBody(codec, existingV1beta1)}, nil case p == "/apis/certificates.k8s.io/v1/certificatesigningrequests/approved" && m == http.MethodGet: return &http.Response{StatusCode: http.StatusOK, Header: cmdtesting.DefaultHeader(), Body: cmdtesting.ObjBody(codec, approvedV1)}, nil case p == "/apis/certificates.k8s.io/v1/certificatesigningrequests/approved/approval" && m == http.MethodPut: return &http.Response{StatusCode: http.StatusOK, Header: cmdtesting.DefaultHeader(), Body: cmdtesting.ObjBody(codec, approvedV1)}, nil - case p == "/apis/certificates.k8s.io/v1beta1/certificatesigningrequests/approved" && m == http.MethodGet: - return &http.Response{StatusCode: http.StatusOK, Header: cmdtesting.DefaultHeader(), Body: cmdtesting.ObjBody(codec, approvedV1beta1)}, nil - case p == "/apis/certificates.k8s.io/v1beta1/certificatesigningrequests/approved/approval" && m == http.MethodPut: - return &http.Response{StatusCode: http.StatusOK, Header: cmdtesting.DefaultHeader(), Body: cmdtesting.ObjBody(codec, approvedV1beta1)}, nil case p == "/apis/certificates.k8s.io/v1/certificatesigningrequests/denied" && m == http.MethodGet: return &http.Response{StatusCode: http.StatusOK, Header: cmdtesting.DefaultHeader(), Body: cmdtesting.ObjBody(codec, deniedV1)}, nil case p == "/apis/certificates.k8s.io/v1/certificatesigningrequests/denied/approval" && m == http.MethodPut: return &http.Response{StatusCode: http.StatusOK, Header: cmdtesting.DefaultHeader(), Body: cmdtesting.ObjBody(codec, deniedV1)}, nil - case p == "/apis/certificates.k8s.io/v1beta1/certificatesigningrequests/denied" && m == http.MethodGet: - return &http.Response{StatusCode: http.StatusOK, Header: cmdtesting.DefaultHeader(), Body: cmdtesting.ObjBody(codec, deniedV1beta1)}, nil - case p == "/apis/certificates.k8s.io/v1beta1/certificatesigningrequests/denied/approval" && m == http.MethodPut: - return &http.Response{StatusCode: http.StatusOK, Header: cmdtesting.DefaultHeader(), Body: cmdtesting.ObjBody(codec, deniedV1beta1)}, nil default: t.Fatalf("unexpected request: %#v\n%#v", req.URL, req)