From 94d077a5fbaa79f5427b6d72d974e13a28ef5fb6 Mon Sep 17 00:00:00 2001 From: JoshVanL Date: Mon, 23 Sep 2019 12:27:00 +0100 Subject: [PATCH 1/6] Adds status sub resource and changes updates Signed-off-by: JoshVanL --- deploy/manifests/00-crds.yaml | 16 ++++++-- pkg/apis/acme/v1alpha2/types_challenge.go | 1 + pkg/apis/acme/v1alpha2/types_order.go | 1 + .../certmanager/v1alpha2/types_certificate.go | 1 + .../v1alpha2/types_certificaterequest.go | 1 + pkg/apis/certmanager/v1alpha2/types_issuer.go | 2 + pkg/controller/acmechallenges/controller.go | 2 +- pkg/controller/acmechallenges/sync.go | 2 +- pkg/controller/acmechallenges/sync_test.go | 29 ++++++++------ pkg/controller/acmeorders/sync.go | 2 +- pkg/controller/acmeorders/sync_test.go | 12 ++++-- .../certificaterequests/acme/acme_test.go | 18 ++++++--- .../certificaterequests/ca/ca_test.go | 15 ++++--- .../selfsigned/selfsigned_test.go | 30 +++++++++----- pkg/controller/certificaterequests/sync.go | 2 +- .../certificaterequests/sync_test.go | 24 ++++++++---- .../certificaterequests/vault/vault_test.go | 21 ++++++---- .../certificaterequests/venafi/venafi_test.go | 36 +++++++++++------ pkg/controller/certificates/sync_test.go | 39 ++++++++++++------- pkg/controller/certificates/util.go | 2 +- pkg/controller/clusterissuers/sync.go | 2 +- pkg/controller/issuers/sync.go | 2 +- 22 files changed, 173 insertions(+), 87 deletions(-) diff --git a/deploy/manifests/00-crds.yaml b/deploy/manifests/00-crds.yaml index cc92399a6..902d99313 100644 --- a/deploy/manifests/00-crds.yaml +++ b/deploy/manifests/00-crds.yaml @@ -29,7 +29,8 @@ spec: kind: Challenge plural: challenges scope: Namespaced - subresources: {} + subresources: + status: {} validation: openAPIV3Schema: description: Challenge is a type to represent a Challenge request with an ACME @@ -1404,7 +1405,8 @@ spec: kind: Order plural: orders scope: Namespaced - subresources: {} + subresources: + status: {} validation: openAPIV3Schema: description: Order is a type to represent an Order with an ACME server @@ -1613,7 +1615,8 @@ spec: - cr - crs scope: Namespaced - subresources: {} + subresources: + status: {} validation: openAPIV3Schema: description: CertificateRequest is a type to represent a Certificate Signing @@ -1803,7 +1806,8 @@ spec: - cert - certs scope: Namespaced - subresources: {} + subresources: + status: {} validation: openAPIV3Schema: description: Certificate is a type to represent a Certificate from ACME @@ -2016,6 +2020,8 @@ spec: kind: ClusterIssuer plural: clusterissuers scope: Cluster + subresources: + status: {} validation: openAPIV3Schema: properties: @@ -3676,6 +3682,8 @@ spec: kind: Issuer plural: issuers scope: Namespaced + subresources: + status: {} validation: openAPIV3Schema: properties: diff --git a/pkg/apis/acme/v1alpha2/types_challenge.go b/pkg/apis/acme/v1alpha2/types_challenge.go index a1285c1a3..815db97bc 100644 --- a/pkg/apis/acme/v1alpha2/types_challenge.go +++ b/pkg/apis/acme/v1alpha2/types_challenge.go @@ -31,6 +31,7 @@ import ( // +kubebuilder:printcolumn:name="Domain",type="string",JSONPath=".spec.dnsName" // +kubebuilder:printcolumn:name="Reason",type="string",JSONPath=".status.reason",description="",priority=1 // +kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp",description="CreationTimestamp is a timestamp representing the server time when this object was created. It is not guaranteed to be set in happens-before order across separate operations. Clients may not set this value. It is represented in RFC3339 form and is in UTC." +// +kubebuilder:subresource:status // +kubebuilder:resource:path=challenges type Challenge struct { metav1.TypeMeta `json:",inline"` diff --git a/pkg/apis/acme/v1alpha2/types_order.go b/pkg/apis/acme/v1alpha2/types_order.go index 00c9eb9be..974e80e2e 100644 --- a/pkg/apis/acme/v1alpha2/types_order.go +++ b/pkg/apis/acme/v1alpha2/types_order.go @@ -34,6 +34,7 @@ import ( // +kubebuilder:printcolumn:name="Issuer",type="string",JSONPath=".spec.issuerRef.name",description="",priority=1 // +kubebuilder:printcolumn:name="Reason",type="string",JSONPath=".status.reason",description="",priority=1 // +kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp",description="CreationTimestamp is a timestamp representing the server time when this object was created. It is not guaranteed to be set in happens-before order across separate operations. Clients may not set this value. It is represented in RFC3339 form and is in UTC." +// +kubebuilder:subresource:status // +kubebuilder:resource:path=orders type Order struct { metav1.TypeMeta `json:",inline"` diff --git a/pkg/apis/certmanager/v1alpha2/types_certificate.go b/pkg/apis/certmanager/v1alpha2/types_certificate.go index 8f073ac88..d2894bd52 100644 --- a/pkg/apis/certmanager/v1alpha2/types_certificate.go +++ b/pkg/apis/certmanager/v1alpha2/types_certificate.go @@ -32,6 +32,7 @@ import ( // +kubebuilder:printcolumn:name="Issuer",type="string",JSONPath=".spec.issuerRef.name",description="",priority=1 // +kubebuilder:printcolumn:name="Status",type="string",JSONPath=".status.conditions[?(@.type==\"Ready\")].message",priority=1 // +kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp",description="CreationTimestamp is a timestamp representing the server time when this object was created. It is not guaranteed to be set in happens-before order across separate operations. Clients may not set this value. It is represented in RFC3339 form and is in UTC." +// +kubebuilder:subresource:status // +kubebuilder:resource:path=certificates,shortName=cert;certs type Certificate struct { metav1.TypeMeta `json:",inline"` diff --git a/pkg/apis/certmanager/v1alpha2/types_certificaterequest.go b/pkg/apis/certmanager/v1alpha2/types_certificaterequest.go index 5130f902b..b8ad1a6c2 100644 --- a/pkg/apis/certmanager/v1alpha2/types_certificaterequest.go +++ b/pkg/apis/certmanager/v1alpha2/types_certificaterequest.go @@ -37,6 +37,7 @@ const ( // +kubebuilder:printcolumn:name="Issuer",type="string",JSONPath=".spec.issuerRef.name",description="",priority=1 // +kubebuilder:printcolumn:name="Status",type="string",JSONPath=".status.conditions[?(@.type==\"Ready\")].message",priority=1 // +kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp",description="CreationTimestamp is a timestamp representing the server time when this object was created. It is not guaranteed to be set in happens-before order across separate operations. Clients may not set this value. It is represented in RFC3339 form and is in UTC." +// +kubebuilder:subresource:status // +kubebuilder:resource:path=certificaterequests,shortName=cr;crs type CertificateRequest struct { metav1.TypeMeta `json:",inline"` diff --git a/pkg/apis/certmanager/v1alpha2/types_issuer.go b/pkg/apis/certmanager/v1alpha2/types_issuer.go index 170018076..75e81eb4e 100644 --- a/pkg/apis/certmanager/v1alpha2/types_issuer.go +++ b/pkg/apis/certmanager/v1alpha2/types_issuer.go @@ -28,6 +28,7 @@ import ( // +k8s:openapi-gen=true // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object +// +kubebuilder:subresource:status // +kubebuilder:resource:path=clusterissuers,scope=Cluster type ClusterIssuer struct { metav1.TypeMeta `json:",inline"` @@ -51,6 +52,7 @@ type ClusterIssuerList struct { // +k8s:openapi-gen=true // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object +// +kubebuilder:subresource:status // +kubebuilder:resource:path=issuers type Issuer struct { metav1.TypeMeta `json:",inline"` diff --git a/pkg/controller/acmechallenges/controller.go b/pkg/controller/acmechallenges/controller.go index 030bdc31b..cd6c68ed3 100644 --- a/pkg/controller/acmechallenges/controller.go +++ b/pkg/controller/acmechallenges/controller.go @@ -164,7 +164,7 @@ func (c *controller) runScheduler(ctx context.Context) { ch = ch.DeepCopy() ch.Status.Processing = true - _, err := c.cmClient.AcmeV1alpha2().Challenges(ch.Namespace).Update(ch) + _, err := c.cmClient.AcmeV1alpha2().Challenges(ch.Namespace).UpdateStatus(ch) if err != nil { log.Error(err, "error scheduling challenge for processing") return diff --git a/pkg/controller/acmechallenges/sync.go b/pkg/controller/acmechallenges/sync.go index f18d9e160..24ddc4820 100644 --- a/pkg/controller/acmechallenges/sync.go +++ b/pkg/controller/acmechallenges/sync.go @@ -70,7 +70,7 @@ func (c *controller) Sync(ctx context.Context, ch *cmacme.Challenge) (err error) if reflect.DeepEqual(oldChal.Status, ch.Status) && len(oldChal.Finalizers) == len(ch.Finalizers) { return } - _, updateErr := c.cmClient.AcmeV1alpha2().Challenges(ch.Namespace).Update(ch) + _, updateErr := c.cmClient.AcmeV1alpha2().Challenges(ch.Namespace).UpdateStatus(ch) if err != nil { err = utilerrors.NewAggregate([]error{err, updateErr}) } diff --git a/pkg/controller/acmechallenges/sync_test.go b/pkg/controller/acmechallenges/sync_test.go index 81dd0cf62..444823dea 100644 --- a/pkg/controller/acmechallenges/sync_test.go +++ b/pkg/controller/acmechallenges/sync_test.go @@ -97,12 +97,14 @@ func TestSyncHappyPath(t *testing.T) { gen.SetChallengeURL("testurl"), ), testIssuerHTTP01Enabled}, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction(cmacme.SchemeGroupVersion.WithResource("challenges"), gen.DefaultTestNamespace, - gen.ChallengeFrom(baseChallenge, - gen.SetChallengeProcessing(true), - gen.SetChallengeURL("testurl"), - gen.SetChallengeState(cmacme.Pending), - ))), + testpkg.NewAction( + coretesting.NewUpdateSubresourceAction(cmacme.SchemeGroupVersion.WithResource("challenges"), "status", + gen.DefaultTestNamespace, + gen.ChallengeFrom(baseChallenge, + gen.SetChallengeProcessing(true), + gen.SetChallengeURL("testurl"), + gen.SetChallengeState(cmacme.Pending), + ))), }, }, acmeClient: &acmecl.FakeACME{ @@ -134,7 +136,8 @@ func TestSyncHappyPath(t *testing.T) { gen.SetChallengeType("http-01"), ), testIssuerHTTP01Enabled}, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction(cmacme.SchemeGroupVersion.WithResource("challenges"), gen.DefaultTestNamespace, + testpkg.NewAction(coretesting.NewUpdateSubresourceAction(cmacme.SchemeGroupVersion.WithResource("challenges"), "status", + gen.DefaultTestNamespace, gen.ChallengeFrom(baseChallenge, gen.SetChallengeProcessing(true), gen.SetChallengeURL("testurl"), @@ -176,7 +179,8 @@ func TestSyncHappyPath(t *testing.T) { gen.SetChallengePresented(true), ), testIssuerHTTP01Enabled}, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction(cmacme.SchemeGroupVersion.WithResource("challenges"), gen.DefaultTestNamespace, + testpkg.NewAction(coretesting.NewUpdateSubresourceAction(cmacme.SchemeGroupVersion.WithResource("challenges"), "status", + gen.DefaultTestNamespace, gen.ChallengeFrom(baseChallenge, gen.SetChallengeProcessing(true), gen.SetChallengeURL("testurl"), @@ -228,7 +232,8 @@ func TestSyncHappyPath(t *testing.T) { gen.SetChallengePresented(true), ), testIssuerHTTP01Enabled}, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction(cmacme.SchemeGroupVersion.WithResource("challenges"), gen.DefaultTestNamespace, + testpkg.NewAction(coretesting.NewUpdateSubresourceAction(cmacme.SchemeGroupVersion.WithResource("challenges"), "status", + gen.DefaultTestNamespace, gen.ChallengeFrom(baseChallenge, gen.SetChallengeProcessing(true), gen.SetChallengeURL("testurl"), @@ -283,7 +288,8 @@ func TestSyncHappyPath(t *testing.T) { gen.SetChallengePresented(true), ), testIssuerHTTP01Enabled}, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction(cmacme.SchemeGroupVersion.WithResource("challenges"), gen.DefaultTestNamespace, + testpkg.NewAction(coretesting.NewUpdateSubresourceAction(cmacme.SchemeGroupVersion.WithResource("challenges"), "status", + gen.DefaultTestNamespace, gen.ChallengeFrom(baseChallenge, gen.SetChallengeProcessing(false), gen.SetChallengeURL("testurl"), @@ -316,7 +322,8 @@ func TestSyncHappyPath(t *testing.T) { gen.SetChallengePresented(true), ), testIssuerHTTP01Enabled}, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction(cmacme.SchemeGroupVersion.WithResource("challenges"), gen.DefaultTestNamespace, + testpkg.NewAction(coretesting.NewUpdateSubresourceAction(cmacme.SchemeGroupVersion.WithResource("challenges"), "status", + gen.DefaultTestNamespace, gen.ChallengeFrom(baseChallenge, gen.SetChallengeProcessing(false), gen.SetChallengeURL("testurl"), diff --git a/pkg/controller/acmeorders/sync.go b/pkg/controller/acmeorders/sync.go index 8cd68c82e..1390e0ccf 100644 --- a/pkg/controller/acmeorders/sync.go +++ b/pkg/controller/acmeorders/sync.go @@ -54,7 +54,7 @@ func (c *controller) Sync(ctx context.Context, o *cmacme.Order) (err error) { return } log.Info("updating Order resource status") - _, updateErr := c.cmClient.AcmeV1alpha2().Orders(o.Namespace).Update(o) + _, updateErr := c.cmClient.AcmeV1alpha2().Orders(o.Namespace).UpdateStatus(o) if err != nil { log.Error(err, "failed to update status") err = utilerrors.NewAggregate([]error{err, updateErr}) diff --git a/pkg/controller/acmeorders/sync_test.go b/pkg/controller/acmeorders/sync_test.go index fa592e341..126f44cf3 100644 --- a/pkg/controller/acmeorders/sync_test.go +++ b/pkg/controller/acmeorders/sync_test.go @@ -153,7 +153,8 @@ dGVzdA== builder: &testpkg.Builder{ CertManagerObjects: []runtime.Object{testIssuerHTTP01TestCom, testOrder}, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction(cmacme.SchemeGroupVersion.WithResource("orders"), testOrderPending.Namespace, + testpkg.NewAction(coretesting.NewUpdateSubresourceAction(cmacme.SchemeGroupVersion.WithResource("orders"), "status", + testOrderPending.Namespace, gen.OrderFrom(testOrder, gen.SetOrderStatus(cmacme.OrderStatus{ State: cmacme.Pending, URL: "http://testurl.com/abcde", @@ -216,7 +217,8 @@ dGVzdA== builder: &testpkg.Builder{ CertManagerObjects: []runtime.Object{testIssuerHTTP01TestCom, testOrderPending, testAuthorizationChallengeValid}, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction(cmacme.SchemeGroupVersion.WithResource("orders"), testOrderReady.Namespace, testOrderReady)), + testpkg.NewAction(coretesting.NewUpdateSubresourceAction(cmacme.SchemeGroupVersion.WithResource("orders"), "status", + testOrderReady.Namespace, testOrderReady)), }, }, acmeClient: &acmecl.FakeACME{ @@ -234,7 +236,8 @@ dGVzdA== builder: &testpkg.Builder{ CertManagerObjects: []runtime.Object{testIssuerHTTP01TestCom, testOrderReady, testAuthorizationChallengeValid}, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction(cmacme.SchemeGroupVersion.WithResource("orders"), testOrderValid.Namespace, testOrderValid)), + testpkg.NewAction(coretesting.NewUpdateSubresourceAction(cmacme.SchemeGroupVersion.WithResource("orders"), "status", + testOrderValid.Namespace, testOrderValid)), }, ExpectedEvents: []string{ "Normal Complete Order completed successfully", @@ -259,7 +262,8 @@ dGVzdA== builder: &testpkg.Builder{ CertManagerObjects: []runtime.Object{testIssuerHTTP01TestCom, testOrderPending, testAuthorizationChallengeInvalid}, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction(cmacme.SchemeGroupVersion.WithResource("orders"), testOrderInvalid.Namespace, testOrderInvalid)), + testpkg.NewAction(coretesting.NewUpdateSubresourceAction(cmacme.SchemeGroupVersion.WithResource("orders"), "status", + testOrderInvalid.Namespace, testOrderInvalid)), }, }, acmeClient: &acmecl.FakeACME{ diff --git a/pkg/controller/certificaterequests/acme/acme_test.go b/pkg/controller/certificaterequests/acme/acme_test.go index f006d86ac..1e65259e4 100644 --- a/pkg/controller/certificaterequests/acme/acme_test.go +++ b/pkg/controller/certificaterequests/acme/acme_test.go @@ -132,8 +132,9 @@ func TestSign(t *testing.T) { "Warning BadConfig Resource validation failed: spec.csr: Invalid value: []byte{0x61, 0x20, 0x62, 0x61, 0x64, 0x20, 0x63, 0x73, 0x72}: failed to decode csr: error decoding certificate request PEM block", }, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction( + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + "status", gen.DefaultTestNamespace, gen.CertificateRequestFrom(baseCR, gen.SetCertificateRequestCSR([]byte("a bad csr")), @@ -166,8 +167,9 @@ func TestSign(t *testing.T) { gen.DefaultTestNamespace, baseOrder, )), - testpkg.NewAction(coretesting.NewUpdateAction( + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + "status", gen.DefaultTestNamespace, gen.CertificateRequestFrom(baseCR, gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{ @@ -191,8 +193,9 @@ func TestSign(t *testing.T) { }, CertManagerObjects: []runtime.Object{baseCR.DeepCopy(), baseIssuer.DeepCopy()}, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction( + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + "status", gen.DefaultTestNamespace, gen.CertificateRequestFrom(baseCR, gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{ @@ -230,8 +233,9 @@ func TestSign(t *testing.T) { ), }, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction( + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + "status", gen.DefaultTestNamespace, gen.CertificateRequestFrom(baseCR, gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{ @@ -260,8 +264,9 @@ func TestSign(t *testing.T) { ), }, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction( + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + "status", gen.DefaultTestNamespace, gen.CertificateRequestFrom(baseCR, gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{ @@ -288,8 +293,9 @@ func TestSign(t *testing.T) { gen.SetOrderCertificate(certPEM), ), baseCR.DeepCopy(), baseIssuer.DeepCopy()}, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction( + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + "status", gen.DefaultTestNamespace, gen.CertificateRequestFrom(baseCR, gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{ diff --git a/pkg/controller/certificaterequests/ca/ca_test.go b/pkg/controller/certificaterequests/ca/ca_test.go index f1f044ec1..584049701 100644 --- a/pkg/controller/certificaterequests/ca/ca_test.go +++ b/pkg/controller/certificaterequests/ca/ca_test.go @@ -159,8 +159,9 @@ func TestSign(t *testing.T) { `Normal SecretMissing Referenced secret default-unit-test-ns/root-ca-secret not found: secret "root-ca-secret" not found`, }, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction( + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + "status", gen.DefaultTestNamespace, gen.CertificateRequestFrom(baseCR.DeepCopy(), gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{ @@ -184,8 +185,9 @@ func TestSign(t *testing.T) { "Normal SecretInvalidData Failed to parse signing CA keypair from secret default-unit-test-ns/root-ca-secret: error decoding private key PEM block", }, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction( + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + "status", gen.DefaultTestNamespace, gen.CertificateRequestFrom(baseCR.DeepCopy(), gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{ @@ -209,8 +211,9 @@ func TestSign(t *testing.T) { `Normal SecretGetError Failed to get certificate key pair from secret default-unit-test-ns/root-ca-secret: this is a network error`, }, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction( + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + "status", gen.DefaultTestNamespace, gen.CertificateRequestFrom(baseCR, gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{ @@ -247,8 +250,9 @@ func TestSign(t *testing.T) { "Warning SigningError Error generating certificate template: this is a sign error", }, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction( + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + "status", gen.DefaultTestNamespace, gen.CertificateRequestFrom(baseCR.DeepCopy(), gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{ @@ -281,8 +285,9 @@ func TestSign(t *testing.T) { "Normal CertificateIssued Certificate fetched from issuer successfully", }, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction( + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + "status", gen.DefaultTestNamespace, gen.CertificateRequestFrom(baseCR, gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{ diff --git a/pkg/controller/certificaterequests/selfsigned/selfsigned_test.go b/pkg/controller/certificaterequests/selfsigned/selfsigned_test.go index 53da69b7f..2c7890594 100644 --- a/pkg/controller/certificaterequests/selfsigned/selfsigned_test.go +++ b/pkg/controller/certificaterequests/selfsigned/selfsigned_test.go @@ -180,8 +180,9 @@ func TestSign(t *testing.T) { `Warning MissingAnnotation Annotation "cert-manager.io/private-key-secret-name" missing or reference empty: secret name missing`, }, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction( + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + "status", gen.DefaultTestNamespace, gen.CertificateRequestFrom(baseCR, gen.SetCertificateRequestAnnotations(map[string]string{}), @@ -213,8 +214,9 @@ func TestSign(t *testing.T) { `Warning MissingAnnotation Annotation "cert-manager.io/private-key-secret-name" missing or reference empty: secret name missing`, }, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction( + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + "status", gen.DefaultTestNamespace, gen.CertificateRequestFrom(baseCR, gen.SetCertificateRequestAnnotations(map[string]string{cmapi.CRPrivateKeyAnnotationKey: ""}), @@ -240,8 +242,9 @@ func TestSign(t *testing.T) { `Normal MissingSecret Referenced secret default-unit-test-ns/test-rsa-key not found: secret "test-rsa-key" not found`, }, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction( + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + "status", gen.DefaultTestNamespace, gen.CertificateRequestFrom(baseCR, gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{ @@ -265,8 +268,9 @@ func TestSign(t *testing.T) { `Normal ErrorParsingKey Failed to get key "test-rsa-key" referenced in annotation "cert-manager.io/private-key-secret-name": error decoding private key PEM block`, }, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction( + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + "status", gen.DefaultTestNamespace, gen.CertificateRequestFrom(baseCR, gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{ @@ -290,8 +294,9 @@ func TestSign(t *testing.T) { `Normal ErrorGettingSecret Failed to get certificate key pair from secret default-unit-test-ns/test-rsa-key: this is a network error`, }, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction( + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + "status", gen.DefaultTestNamespace, gen.CertificateRequestFrom(baseCR, gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{ @@ -329,8 +334,9 @@ func TestSign(t *testing.T) { "Warning BadConfig Resource validation failed: spec.csr: Invalid value: []byte{0x74, 0x68, 0x69, 0x73, 0x20, 0x69, 0x73, 0x20, 0x61, 0x20, 0x62, 0x61, 0x64, 0x20, 0x43, 0x53, 0x52}: failed to decode csr: error decoding certificate request PEM block", }, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction( + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + "status", gen.DefaultTestNamespace, gen.CertificateRequestFrom(baseCR, gen.SetCertificateRequestCSR([]byte("this is a bad CSR")), @@ -356,8 +362,9 @@ func TestSign(t *testing.T) { "Warning ErrorKeyMatch Error generating certificate template: CSR not signed by referenced private key", }, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction( + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + "status", gen.DefaultTestNamespace, gen.CertificateRequestFrom(ecCR, gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{ @@ -385,8 +392,9 @@ func TestSign(t *testing.T) { "Warning ErrorSigning Error signing certificate: this is a signing error", }, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction( + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + "status", gen.DefaultTestNamespace, gen.CertificateRequestFrom(baseCR, gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{ @@ -421,8 +429,9 @@ func TestSign(t *testing.T) { "Normal CertificateIssued Certificate fetched from issuer successfully", }, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction( + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + "status", gen.DefaultTestNamespace, gen.CertificateRequestFrom(baseCR, gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{ @@ -458,8 +467,9 @@ func TestSign(t *testing.T) { "Normal CertificateIssued Certificate fetched from issuer successfully", }, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction( + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + "status", gen.DefaultTestNamespace, gen.CertificateRequestFrom(ecCR, gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{ diff --git a/pkg/controller/certificaterequests/sync.go b/pkg/controller/certificaterequests/sync.go index 56cc917b6..a2e5b216d 100644 --- a/pkg/controller/certificaterequests/sync.go +++ b/pkg/controller/certificaterequests/sync.go @@ -160,5 +160,5 @@ func (c *Controller) updateCertificateRequestStatus(ctx context.Context, old, ne // TODO: replace Update call with UpdateStatus. This requires a custom API // server with the /status subresource enabled and/or subresource support // for CRDs (https://github.com/kubernetes/kubernetes/issues/38113) - return c.cmClient.CertmanagerV1alpha2().CertificateRequests(new.Namespace).Update(new) + return c.cmClient.CertmanagerV1alpha2().CertificateRequests(new.Namespace).UpdateStatus(new) } diff --git a/pkg/controller/certificaterequests/sync_test.go b/pkg/controller/certificaterequests/sync_test.go index 5af96beac..956c364a9 100644 --- a/pkg/controller/certificaterequests/sync_test.go +++ b/pkg/controller/certificaterequests/sync_test.go @@ -188,8 +188,9 @@ func TestSync(t *testing.T) { `Normal IssuerNotFound Referenced "Issuer" not found: issuer.cert-manager.io "test-issuer" not found`, }, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction( + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + "status", gen.DefaultTestNamespace, gen.CertificateRequestFrom(baseCR, gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{ @@ -231,8 +232,9 @@ func TestSync(t *testing.T) { ), }, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction( + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + "status", gen.DefaultTestNamespace, gen.CertificateRequestFrom(baseCR, gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{ @@ -276,8 +278,9 @@ func TestSync(t *testing.T) { "Warning BadConfig Resource validation failed: spec.csr: Invalid value: []byte{0x62, 0x61, 0x64, 0x20, 0x63, 0x73, 0x72}: failed to decode csr: error decoding certificate request PEM block", }, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction( + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + "status", gen.DefaultTestNamespace, gen.CertificateRequestFrom(baseCR, gen.SetCertificateRequestCSR([]byte("bad csr")), @@ -350,8 +353,9 @@ func TestSync(t *testing.T) { "Warning DecodeError Failed to decode returned certificate: error decoding cert PEM block", }, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction( + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + "status", gen.DefaultTestNamespace, gen.CertificateRequestFrom(baseCR, gen.SetCertificateRequestCertificate([]byte("a bad certificate")), @@ -383,8 +387,9 @@ func TestSync(t *testing.T) { "Normal CertificateIssued Certificate fetched from issuer successfully", }, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction( + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + "status", gen.DefaultTestNamespace, gen.CertificateRequestFrom(baseCR, gen.SetCertificateRequestCertificate(certRSAPEM), @@ -415,8 +420,9 @@ func TestSync(t *testing.T) { "Normal CertificateIssued Certificate fetched from issuer successfully", }, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction( + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + "status", gen.DefaultTestNamespace, gen.CertificateRequestFrom(baseCR, gen.SetCertificateRequestCertificate(certRSAPEMExpired), @@ -447,8 +453,9 @@ func TestSync(t *testing.T) { "Normal CertificateIssued Certificate fetched from issuer successfully", }, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction( + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + "status", gen.DefaultTestNamespace, gen.CertificateRequestFrom(baseCR, gen.SetCertificateRequestCertificate(certECPEM), @@ -479,8 +486,9 @@ func TestSync(t *testing.T) { "Normal CertificateIssued Certificate fetched from issuer successfully", }, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction( + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + "status", gen.DefaultTestNamespace, gen.CertificateRequestFrom(baseCR, gen.SetCertificateRequestCertificate(certECPEMExpired), diff --git a/pkg/controller/certificaterequests/vault/vault_test.go b/pkg/controller/certificaterequests/vault/vault_test.go index 6845ab97a..6220da087 100644 --- a/pkg/controller/certificaterequests/vault/vault_test.go +++ b/pkg/controller/certificaterequests/vault/vault_test.go @@ -156,8 +156,9 @@ func TestSign(t *testing.T) { "Normal VaultInitError Failed to initialise vault client for signing: error initializing Vault client: tokenSecretRef, appRoleSecretRef, or Kubernetes auth role not set", }, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction( + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + "status", gen.DefaultTestNamespace, gen.CertificateRequestFrom(baseCR, gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{ @@ -192,8 +193,9 @@ func TestSign(t *testing.T) { `Normal SecretMissing Required secret resource not found: secret "non-existing-secret" not found`, }, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction( + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + "status", gen.DefaultTestNamespace, gen.CertificateRequestFrom(baseCR, gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{ @@ -231,8 +233,9 @@ func TestSign(t *testing.T) { `Normal SecretMissing Required secret resource not found: secret "non-existing-secret" not found`, }, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction( + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + "status", gen.DefaultTestNamespace, gen.CertificateRequestFrom(baseCR, gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{ @@ -267,8 +270,9 @@ func TestSign(t *testing.T) { "Warning SigningError Vault failed to sign certificate: failed to sign", }, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction( + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + "status", gen.DefaultTestNamespace, gen.CertificateRequestFrom(baseCR, gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{ @@ -308,8 +312,9 @@ func TestSign(t *testing.T) { `Warning SigningError Vault failed to sign certificate: failed to sign`, }, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction( + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + "status", gen.DefaultTestNamespace, gen.CertificateRequestFrom(baseCR, gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{ @@ -346,8 +351,9 @@ func TestSign(t *testing.T) { "Normal CertificateIssued Certificate fetched from issuer successfully", }, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction( + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + "status", gen.DefaultTestNamespace, gen.CertificateRequestFrom(baseCR, gen.SetCertificateRequestCertificate(rsaPEMCert), @@ -388,8 +394,9 @@ func TestSign(t *testing.T) { "Normal CertificateIssued Certificate fetched from issuer successfully", }, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction( + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + "status", gen.DefaultTestNamespace, gen.CertificateRequestFrom(baseCR, gen.SetCertificateRequestCertificate(rsaPEMCert), diff --git a/pkg/controller/certificaterequests/venafi/venafi_test.go b/pkg/controller/certificaterequests/venafi/venafi_test.go index 8c082936e..7415b5518 100644 --- a/pkg/controller/certificaterequests/venafi/venafi_test.go +++ b/pkg/controller/certificaterequests/venafi/venafi_test.go @@ -208,8 +208,9 @@ func TestSign(t *testing.T) { `Normal SecretMissing Required secret resource not found: secret "test-tpp-secret" not found`, }, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction( + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + "status", gen.DefaultTestNamespace, gen.CertificateRequestFrom(tppCR, gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{ @@ -233,8 +234,9 @@ func TestSign(t *testing.T) { `Normal VenafiInitError Failed to initialise venafi client for signing: this is a network error`, }, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction( + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + "status", gen.DefaultTestNamespace, gen.CertificateRequestFrom(tppCR, gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{ @@ -260,8 +262,9 @@ func TestSign(t *testing.T) { `Normal SecretMissing Required secret resource not found: secret "test-cloud-secret" not found`, }, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction( + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + "status", gen.DefaultTestNamespace, gen.CertificateRequestFrom(cloudCR, gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{ @@ -284,8 +287,9 @@ func TestSign(t *testing.T) { `Normal VenafiInitError Failed to initialise venafi client for signing: this is a network error`, }, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction( + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + "status", gen.DefaultTestNamespace, gen.CertificateRequestFrom(cloudCR, gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{ @@ -311,8 +315,9 @@ func TestSign(t *testing.T) { "Normal IssuancePending Venafi certificate still in a pending state, the request will be retried: Issuance is pending. You may try retrieving the certificate later using Pickup ID: test-cert-id\n\tStatus: test-status-pending", }, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction( + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + "status", gen.DefaultTestNamespace, gen.CertificateRequestFrom(tppCR, gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{ @@ -339,8 +344,9 @@ func TestSign(t *testing.T) { "Normal IssuancePending Venafi certificate still in a pending state, the request will be retried: Issuance is pending. You may try retrieving the certificate later using Pickup ID: test-cert-id\n\tStatus: test-status-pending", }, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction( + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + "status", gen.DefaultTestNamespace, gen.CertificateRequestFrom(cloudCR, gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{ @@ -367,8 +373,9 @@ func TestSign(t *testing.T) { "Warning Timeout Timed out waiting for venafi certificate, the request will be retried: Operation timed out. You may try retrieving the certificate later using Pickup ID: test-cert-id", }, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction( + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + "status", gen.DefaultTestNamespace, gen.CertificateRequestFrom(tppCR, gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{ @@ -395,8 +402,9 @@ func TestSign(t *testing.T) { "Warning Timeout Timed out waiting for venafi certificate, the request will be retried: Operation timed out. You may try retrieving the certificate later using Pickup ID: test-cert-id", }, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction( + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + "status", gen.DefaultTestNamespace, gen.CertificateRequestFrom(cloudCR, gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{ @@ -423,8 +431,9 @@ func TestSign(t *testing.T) { "Warning RetrieveError Failed to obtain venafi certificate: this is an error", }, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction( + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + "status", gen.DefaultTestNamespace, gen.CertificateRequestFrom(tppCR, gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{ @@ -452,8 +461,9 @@ func TestSign(t *testing.T) { "Warning RetrieveError Failed to obtain venafi certificate: this is an error", }, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction( + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + "status", gen.DefaultTestNamespace, gen.CertificateRequestFrom(cloudCR, gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{ @@ -481,8 +491,9 @@ func TestSign(t *testing.T) { "Normal CertificateIssued Certificate fetched from issuer successfully", }, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction( + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + "status", gen.DefaultTestNamespace, gen.CertificateRequestFrom(tppCR, gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{ @@ -509,8 +520,9 @@ func TestSign(t *testing.T) { "Normal CertificateIssued Certificate fetched from issuer successfully", }, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction( + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( cmapi.SchemeGroupVersion.WithResource("certificaterequests"), + "status", gen.DefaultTestNamespace, gen.CertificateRequestFrom(cloudCR, gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{ diff --git a/pkg/controller/certificates/sync_test.go b/pkg/controller/certificates/sync_test.go index 24f9cb66e..2e0e034c3 100644 --- a/pkg/controller/certificates/sync_test.go +++ b/pkg/controller/certificates/sync_test.go @@ -1603,8 +1603,9 @@ func TestUpdateStatus(t *testing.T) { exampleBundle1.certificate, }, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction( + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( cmapi.SchemeGroupVersion.WithResource("certificates"), + "status", gen.DefaultTestNamespace, gen.CertificateFrom(exampleBundle1.certificate, gen.SetCertificateStatusCondition(cmapi.CertificateCondition{ @@ -1635,8 +1636,9 @@ func TestUpdateStatus(t *testing.T) { exampleBundle1.certificate, }, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction( + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( cmapi.SchemeGroupVersion.WithResource("certificates"), + "status", gen.DefaultTestNamespace, gen.CertificateFrom(exampleBundle1.certificate, gen.SetCertificateStatusCondition(cmapi.CertificateCondition{ @@ -1669,8 +1671,9 @@ func TestUpdateStatus(t *testing.T) { exampleBundle1.certificate, }, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction( + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( cmapi.SchemeGroupVersion.WithResource("certificates"), + "status", gen.DefaultTestNamespace, gen.CertificateFrom(exampleBundle1.certificate, gen.SetCertificateStatusCondition(cmapi.CertificateCondition{ @@ -1704,8 +1707,9 @@ func TestUpdateStatus(t *testing.T) { exampleBundle1.certificateRequest, }, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction( + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( cmapi.SchemeGroupVersion.WithResource("certificates"), + "status", gen.DefaultTestNamespace, gen.CertificateFrom(exampleBundle1.certificate, gen.SetCertificateStatusCondition(cmapi.CertificateCondition{ @@ -1743,8 +1747,9 @@ func TestUpdateStatus(t *testing.T) { exampleBundle1.certificate, }, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction( + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( cmapi.SchemeGroupVersion.WithResource("certificates"), + "status", gen.DefaultTestNamespace, gen.CertificateFrom(exampleBundle1.certificate, gen.SetCertificateStatusCondition(cmapi.CertificateCondition{ @@ -1784,8 +1789,9 @@ func TestUpdateStatus(t *testing.T) { exampleBundle1.certificateRequest, }, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction( + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( cmapi.SchemeGroupVersion.WithResource("certificates"), + "status", gen.DefaultTestNamespace, gen.CertificateFrom(exampleBundle1.certificate, gen.SetCertificateStatusCondition(cmapi.CertificateCondition{ @@ -1824,8 +1830,9 @@ func TestUpdateStatus(t *testing.T) { exampleBundle1.certificate, }, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction( + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( cmapi.SchemeGroupVersion.WithResource("certificates"), + "status", gen.DefaultTestNamespace, gen.CertificateFrom(exampleBundle1.certificate, gen.SetCertificateStatusCondition(cmapi.CertificateCondition{ @@ -1865,8 +1872,9 @@ func TestUpdateStatus(t *testing.T) { exampleBundle1.certificateRequest, }, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction( + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( cmapi.SchemeGroupVersion.WithResource("certificates"), + "status", gen.DefaultTestNamespace, gen.CertificateFrom(exampleBundle1.certificate, gen.SetCertificateStatusCondition(cmapi.CertificateCondition{ @@ -1906,8 +1914,9 @@ func TestUpdateStatus(t *testing.T) { exampleBundle1.certificateRequestReady, }, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction( + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( cmapi.SchemeGroupVersion.WithResource("certificates"), + "status", gen.DefaultTestNamespace, gen.CertificateFrom(exampleBundle1.certificate, gen.SetCertificateStatusCondition(cmapi.CertificateCondition{ @@ -1950,8 +1959,9 @@ func TestUpdateStatus(t *testing.T) { exampleBundle1.certificate, }, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction( + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( cmapi.SchemeGroupVersion.WithResource("certificates"), + "status", gen.DefaultTestNamespace, gen.CertificateFrom(exampleBundle1.certificate, gen.SetCertificateStatusCondition(cmapi.CertificateCondition{ @@ -1994,8 +2004,9 @@ func TestUpdateStatus(t *testing.T) { exampleBundle1.certificateRequest, }, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction( + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( cmapi.SchemeGroupVersion.WithResource("certificates"), + "status", gen.DefaultTestNamespace, gen.CertificateFrom(exampleBundle1.certificate, gen.SetCertificateStatusCondition(cmapi.CertificateCondition{ @@ -2033,8 +2044,9 @@ func TestUpdateStatus(t *testing.T) { exampleBundle1.certificate, }, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction( + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( cmapi.SchemeGroupVersion.WithResource("certificates"), + "status", gen.DefaultTestNamespace, gen.CertificateFrom(exampleBundle1.certificate, gen.SetCertificateStatusCondition(cmapi.CertificateCondition{ @@ -2073,8 +2085,9 @@ func TestUpdateStatus(t *testing.T) { exampleBundle1.certificateRequest, }, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction( + testpkg.NewAction(coretesting.NewUpdateSubresourceAction( cmapi.SchemeGroupVersion.WithResource("certificates"), + "status", gen.DefaultTestNamespace, gen.CertificateFrom(exampleBundle1.certificate, gen.SetCertificateStatusCondition(cmapi.CertificateCondition{ diff --git a/pkg/controller/certificates/util.go b/pkg/controller/certificates/util.go index 70485746c..6149ecbf0 100644 --- a/pkg/controller/certificates/util.go +++ b/pkg/controller/certificates/util.go @@ -225,7 +225,7 @@ func updateCertificateStatus(ctx context.Context, m *metrics.Metrics, cmClient c // TODO: replace Update call with UpdateStatus. This requires a custom API // server with the /status subresource enabled and/or subresource support // for CRDs (https://github.com/kubernetes/kubernetes/issues/38113) - return cmClient.CertmanagerV1alpha2().Certificates(new.Namespace).Update(new) + return cmClient.CertmanagerV1alpha2().Certificates(new.Namespace).UpdateStatus(new) } func certificateHasTemporaryCertificateAnnotation(crt *v1alpha2.Certificate) bool { diff --git a/pkg/controller/clusterissuers/sync.go b/pkg/controller/clusterissuers/sync.go index 066a85fa3..47658aa2c 100644 --- a/pkg/controller/clusterissuers/sync.go +++ b/pkg/controller/clusterissuers/sync.go @@ -92,5 +92,5 @@ func (c *controller) updateIssuerStatus(old, new *v1alpha2.ClusterIssuer) (*v1al // TODO: replace Update call with UpdateStatus. This requires a custom API // server with the /status subresource enabled and/or subresource support // for CRDs (https://github.com/kubernetes/kubernetes/issues/38113) - return c.cmClient.CertmanagerV1alpha2().ClusterIssuers().Update(new) + return c.cmClient.CertmanagerV1alpha2().ClusterIssuers().UpdateStatus(new) } diff --git a/pkg/controller/issuers/sync.go b/pkg/controller/issuers/sync.go index 64564ccae..699a16b46 100644 --- a/pkg/controller/issuers/sync.go +++ b/pkg/controller/issuers/sync.go @@ -92,5 +92,5 @@ func (c *controller) updateIssuerStatus(old, new *v1alpha2.Issuer) (*v1alpha2.Is // TODO: replace Update call with UpdateStatus. This requires a custom API // server with the /status subresource enabled and/or subresource support // for CRDs (https://github.com/kubernetes/kubernetes/issues/38113) - return c.cmClient.CertmanagerV1alpha2().Issuers(new.Namespace).Update(new) + return c.cmClient.CertmanagerV1alpha2().Issuers(new.Namespace).UpdateStatus(new) } From de07651142006ffc63bc388c0cc011e501e4ccb0 Mon Sep 17 00:00:00 2001 From: JoshVanL Date: Mon, 23 Sep 2019 13:00:36 +0100 Subject: [PATCH 2/6] Remote updatestatus todos and add minimal version to docs Signed-off-by: JoshVanL --- docs/getting-started/install/kubernetes.rst | 5 +++++ pkg/controller/certificaterequests/sync.go | 3 --- pkg/controller/certificates/util.go | 3 --- pkg/controller/clusterissuers/sync.go | 3 --- pkg/controller/issuers/sync.go | 3 --- 5 files changed, 5 insertions(+), 12 deletions(-) diff --git a/docs/getting-started/install/kubernetes.rst b/docs/getting-started/install/kubernetes.rst index a153c403f..19b700235 100644 --- a/docs/getting-started/install/kubernetes.rst +++ b/docs/getting-started/install/kubernetes.rst @@ -14,6 +14,11 @@ resources which represent certificate authorities. More information on configuring different Issuer types can be found in the :doc:`respective setup guides `. +.. note:: + From cert-manager v0.11.0 onwards, the minimum supported version of + Kubernetes is v1.11.0. Users still running Kubernetes v1.10 or below should + upgrade to a supported version before installing cert-manager. + .. warning:: You should not install multiple instances of cert-manager on a single diff --git a/pkg/controller/certificaterequests/sync.go b/pkg/controller/certificaterequests/sync.go index a2e5b216d..32cfc2380 100644 --- a/pkg/controller/certificaterequests/sync.go +++ b/pkg/controller/certificaterequests/sync.go @@ -157,8 +157,5 @@ func (c *Controller) updateCertificateRequestStatus(ctx context.Context, old, ne } log.V(logf.DebugLevel).Info("updating resource due to change in status", "diff", pretty.Diff(string(oldBytes), string(newBytes))) - // TODO: replace Update call with UpdateStatus. This requires a custom API - // server with the /status subresource enabled and/or subresource support - // for CRDs (https://github.com/kubernetes/kubernetes/issues/38113) return c.cmClient.CertmanagerV1alpha2().CertificateRequests(new.Namespace).UpdateStatus(new) } diff --git a/pkg/controller/certificates/util.go b/pkg/controller/certificates/util.go index 6149ecbf0..f9c3d62d5 100644 --- a/pkg/controller/certificates/util.go +++ b/pkg/controller/certificates/util.go @@ -222,9 +222,6 @@ func updateCertificateStatus(ctx context.Context, m *metrics.Metrics, cmClient c return nil, nil } log.V(logf.DebugLevel).Info("updating resource due to change in status", "diff", pretty.Diff(string(oldBytes), string(newBytes))) - // TODO: replace Update call with UpdateStatus. This requires a custom API - // server with the /status subresource enabled and/or subresource support - // for CRDs (https://github.com/kubernetes/kubernetes/issues/38113) return cmClient.CertmanagerV1alpha2().Certificates(new.Namespace).UpdateStatus(new) } diff --git a/pkg/controller/clusterissuers/sync.go b/pkg/controller/clusterissuers/sync.go index 47658aa2c..088555e37 100644 --- a/pkg/controller/clusterissuers/sync.go +++ b/pkg/controller/clusterissuers/sync.go @@ -89,8 +89,5 @@ func (c *controller) updateIssuerStatus(old, new *v1alpha2.ClusterIssuer) (*v1al if reflect.DeepEqual(old.Status, new.Status) { return nil, nil } - // TODO: replace Update call with UpdateStatus. This requires a custom API - // server with the /status subresource enabled and/or subresource support - // for CRDs (https://github.com/kubernetes/kubernetes/issues/38113) return c.cmClient.CertmanagerV1alpha2().ClusterIssuers().UpdateStatus(new) } diff --git a/pkg/controller/issuers/sync.go b/pkg/controller/issuers/sync.go index 699a16b46..475cf48e1 100644 --- a/pkg/controller/issuers/sync.go +++ b/pkg/controller/issuers/sync.go @@ -89,8 +89,5 @@ func (c *controller) updateIssuerStatus(old, new *v1alpha2.Issuer) (*v1alpha2.Is if reflect.DeepEqual(old.Status, new.Status) { return nil, nil } - // TODO: replace Update call with UpdateStatus. This requires a custom API - // server with the /status subresource enabled and/or subresource support - // for CRDs (https://github.com/kubernetes/kubernetes/issues/38113) return c.cmClient.CertmanagerV1alpha2().Issuers(new.Namespace).UpdateStatus(new) } From b6803a21856756711b5d0620d55923038e3b49e0 Mon Sep 17 00:00:00 2001 From: JoshVanL Date: Mon, 23 Sep 2019 18:03:35 +0100 Subject: [PATCH 3/6] Fully update challenge and order in controller sync instead of UpdateStatus Signed-off-by: JoshVanL --- pkg/controller/acmechallenges/sync.go | 2 +- pkg/controller/acmechallenges/sync_test.go | 12 ++++++------ pkg/controller/acmeorders/sync.go | 2 +- pkg/controller/acmeorders/sync_test.go | 8 ++++---- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/pkg/controller/acmechallenges/sync.go b/pkg/controller/acmechallenges/sync.go index 24ddc4820..f18d9e160 100644 --- a/pkg/controller/acmechallenges/sync.go +++ b/pkg/controller/acmechallenges/sync.go @@ -70,7 +70,7 @@ func (c *controller) Sync(ctx context.Context, ch *cmacme.Challenge) (err error) if reflect.DeepEqual(oldChal.Status, ch.Status) && len(oldChal.Finalizers) == len(ch.Finalizers) { return } - _, updateErr := c.cmClient.AcmeV1alpha2().Challenges(ch.Namespace).UpdateStatus(ch) + _, updateErr := c.cmClient.AcmeV1alpha2().Challenges(ch.Namespace).Update(ch) if err != nil { err = utilerrors.NewAggregate([]error{err, updateErr}) } diff --git a/pkg/controller/acmechallenges/sync_test.go b/pkg/controller/acmechallenges/sync_test.go index 444823dea..e3df6d3d1 100644 --- a/pkg/controller/acmechallenges/sync_test.go +++ b/pkg/controller/acmechallenges/sync_test.go @@ -98,7 +98,7 @@ func TestSyncHappyPath(t *testing.T) { ), testIssuerHTTP01Enabled}, ExpectedActions: []testpkg.Action{ testpkg.NewAction( - coretesting.NewUpdateSubresourceAction(cmacme.SchemeGroupVersion.WithResource("challenges"), "status", + coretesting.NewUpdateAction(cmacme.SchemeGroupVersion.WithResource("challenges"), gen.DefaultTestNamespace, gen.ChallengeFrom(baseChallenge, gen.SetChallengeProcessing(true), @@ -136,7 +136,7 @@ func TestSyncHappyPath(t *testing.T) { gen.SetChallengeType("http-01"), ), testIssuerHTTP01Enabled}, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateSubresourceAction(cmacme.SchemeGroupVersion.WithResource("challenges"), "status", + testpkg.NewAction(coretesting.NewUpdateAction(cmacme.SchemeGroupVersion.WithResource("challenges"), gen.DefaultTestNamespace, gen.ChallengeFrom(baseChallenge, gen.SetChallengeProcessing(true), @@ -179,7 +179,7 @@ func TestSyncHappyPath(t *testing.T) { gen.SetChallengePresented(true), ), testIssuerHTTP01Enabled}, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateSubresourceAction(cmacme.SchemeGroupVersion.WithResource("challenges"), "status", + testpkg.NewAction(coretesting.NewUpdateAction(cmacme.SchemeGroupVersion.WithResource("challenges"), gen.DefaultTestNamespace, gen.ChallengeFrom(baseChallenge, gen.SetChallengeProcessing(true), @@ -232,7 +232,7 @@ func TestSyncHappyPath(t *testing.T) { gen.SetChallengePresented(true), ), testIssuerHTTP01Enabled}, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateSubresourceAction(cmacme.SchemeGroupVersion.WithResource("challenges"), "status", + testpkg.NewAction(coretesting.NewUpdateAction(cmacme.SchemeGroupVersion.WithResource("challenges"), gen.DefaultTestNamespace, gen.ChallengeFrom(baseChallenge, gen.SetChallengeProcessing(true), @@ -288,7 +288,7 @@ func TestSyncHappyPath(t *testing.T) { gen.SetChallengePresented(true), ), testIssuerHTTP01Enabled}, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateSubresourceAction(cmacme.SchemeGroupVersion.WithResource("challenges"), "status", + testpkg.NewAction(coretesting.NewUpdateAction(cmacme.SchemeGroupVersion.WithResource("challenges"), gen.DefaultTestNamespace, gen.ChallengeFrom(baseChallenge, gen.SetChallengeProcessing(false), @@ -322,7 +322,7 @@ func TestSyncHappyPath(t *testing.T) { gen.SetChallengePresented(true), ), testIssuerHTTP01Enabled}, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateSubresourceAction(cmacme.SchemeGroupVersion.WithResource("challenges"), "status", + testpkg.NewAction(coretesting.NewUpdateAction(cmacme.SchemeGroupVersion.WithResource("challenges"), gen.DefaultTestNamespace, gen.ChallengeFrom(baseChallenge, gen.SetChallengeProcessing(false), diff --git a/pkg/controller/acmeorders/sync.go b/pkg/controller/acmeorders/sync.go index 1390e0ccf..8cd68c82e 100644 --- a/pkg/controller/acmeorders/sync.go +++ b/pkg/controller/acmeorders/sync.go @@ -54,7 +54,7 @@ func (c *controller) Sync(ctx context.Context, o *cmacme.Order) (err error) { return } log.Info("updating Order resource status") - _, updateErr := c.cmClient.AcmeV1alpha2().Orders(o.Namespace).UpdateStatus(o) + _, updateErr := c.cmClient.AcmeV1alpha2().Orders(o.Namespace).Update(o) if err != nil { log.Error(err, "failed to update status") err = utilerrors.NewAggregate([]error{err, updateErr}) diff --git a/pkg/controller/acmeorders/sync_test.go b/pkg/controller/acmeorders/sync_test.go index 126f44cf3..2a8cdd941 100644 --- a/pkg/controller/acmeorders/sync_test.go +++ b/pkg/controller/acmeorders/sync_test.go @@ -153,7 +153,7 @@ dGVzdA== builder: &testpkg.Builder{ CertManagerObjects: []runtime.Object{testIssuerHTTP01TestCom, testOrder}, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateSubresourceAction(cmacme.SchemeGroupVersion.WithResource("orders"), "status", + testpkg.NewAction(coretesting.NewUpdateAction(cmacme.SchemeGroupVersion.WithResource("orders"), testOrderPending.Namespace, gen.OrderFrom(testOrder, gen.SetOrderStatus(cmacme.OrderStatus{ State: cmacme.Pending, @@ -217,7 +217,7 @@ dGVzdA== builder: &testpkg.Builder{ CertManagerObjects: []runtime.Object{testIssuerHTTP01TestCom, testOrderPending, testAuthorizationChallengeValid}, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateSubresourceAction(cmacme.SchemeGroupVersion.WithResource("orders"), "status", + testpkg.NewAction(coretesting.NewUpdateAction(cmacme.SchemeGroupVersion.WithResource("orders"), testOrderReady.Namespace, testOrderReady)), }, }, @@ -236,7 +236,7 @@ dGVzdA== builder: &testpkg.Builder{ CertManagerObjects: []runtime.Object{testIssuerHTTP01TestCom, testOrderReady, testAuthorizationChallengeValid}, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateSubresourceAction(cmacme.SchemeGroupVersion.WithResource("orders"), "status", + testpkg.NewAction(coretesting.NewUpdateAction(cmacme.SchemeGroupVersion.WithResource("orders"), testOrderValid.Namespace, testOrderValid)), }, ExpectedEvents: []string{ @@ -262,7 +262,7 @@ dGVzdA== builder: &testpkg.Builder{ CertManagerObjects: []runtime.Object{testIssuerHTTP01TestCom, testOrderPending, testAuthorizationChallengeInvalid}, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateSubresourceAction(cmacme.SchemeGroupVersion.WithResource("orders"), "status", + testpkg.NewAction(coretesting.NewUpdateAction(cmacme.SchemeGroupVersion.WithResource("orders"), testOrderInvalid.Namespace, testOrderInvalid)), }, }, From 8bf08237f77ca9dfb60ac275028f56c962e5cd9e Mon Sep 17 00:00:00 2001 From: James Munnelly Date: Wed, 2 Oct 2019 12:32:43 +0100 Subject: [PATCH 4/6] Migrate acmechallenges and acmeorders to use UpdateStatus Signed-off-by: James Munnelly --- pkg/controller/acmechallenges/sync.go | 2 +- pkg/controller/acmechallenges/sync_test.go | 18 ++++++++++++------ pkg/controller/acmeorders/sync.go | 2 +- pkg/controller/acmeorders/sync_test.go | 12 ++++++++---- 4 files changed, 22 insertions(+), 12 deletions(-) diff --git a/pkg/controller/acmechallenges/sync.go b/pkg/controller/acmechallenges/sync.go index f18d9e160..24ddc4820 100644 --- a/pkg/controller/acmechallenges/sync.go +++ b/pkg/controller/acmechallenges/sync.go @@ -70,7 +70,7 @@ func (c *controller) Sync(ctx context.Context, ch *cmacme.Challenge) (err error) if reflect.DeepEqual(oldChal.Status, ch.Status) && len(oldChal.Finalizers) == len(ch.Finalizers) { return } - _, updateErr := c.cmClient.AcmeV1alpha2().Challenges(ch.Namespace).Update(ch) + _, updateErr := c.cmClient.AcmeV1alpha2().Challenges(ch.Namespace).UpdateStatus(ch) if err != nil { err = utilerrors.NewAggregate([]error{err, updateErr}) } diff --git a/pkg/controller/acmechallenges/sync_test.go b/pkg/controller/acmechallenges/sync_test.go index e3df6d3d1..b3f4fe482 100644 --- a/pkg/controller/acmechallenges/sync_test.go +++ b/pkg/controller/acmechallenges/sync_test.go @@ -98,7 +98,8 @@ func TestSyncHappyPath(t *testing.T) { ), testIssuerHTTP01Enabled}, ExpectedActions: []testpkg.Action{ testpkg.NewAction( - coretesting.NewUpdateAction(cmacme.SchemeGroupVersion.WithResource("challenges"), + coretesting.NewUpdateSubresourceAction(cmacme.SchemeGroupVersion.WithResource("challenges"), + "status", gen.DefaultTestNamespace, gen.ChallengeFrom(baseChallenge, gen.SetChallengeProcessing(true), @@ -136,7 +137,8 @@ func TestSyncHappyPath(t *testing.T) { gen.SetChallengeType("http-01"), ), testIssuerHTTP01Enabled}, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction(cmacme.SchemeGroupVersion.WithResource("challenges"), + testpkg.NewAction(coretesting.NewUpdateSubresourceAction(cmacme.SchemeGroupVersion.WithResource("challenges"), + "status", gen.DefaultTestNamespace, gen.ChallengeFrom(baseChallenge, gen.SetChallengeProcessing(true), @@ -179,7 +181,8 @@ func TestSyncHappyPath(t *testing.T) { gen.SetChallengePresented(true), ), testIssuerHTTP01Enabled}, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction(cmacme.SchemeGroupVersion.WithResource("challenges"), + testpkg.NewAction(coretesting.NewUpdateSubresourceAction(cmacme.SchemeGroupVersion.WithResource("challenges"), + "status", gen.DefaultTestNamespace, gen.ChallengeFrom(baseChallenge, gen.SetChallengeProcessing(true), @@ -232,7 +235,8 @@ func TestSyncHappyPath(t *testing.T) { gen.SetChallengePresented(true), ), testIssuerHTTP01Enabled}, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction(cmacme.SchemeGroupVersion.WithResource("challenges"), + testpkg.NewAction(coretesting.NewUpdateSubresourceAction(cmacme.SchemeGroupVersion.WithResource("challenges"), + "status", gen.DefaultTestNamespace, gen.ChallengeFrom(baseChallenge, gen.SetChallengeProcessing(true), @@ -288,7 +292,8 @@ func TestSyncHappyPath(t *testing.T) { gen.SetChallengePresented(true), ), testIssuerHTTP01Enabled}, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction(cmacme.SchemeGroupVersion.WithResource("challenges"), + testpkg.NewAction(coretesting.NewUpdateSubresourceAction(cmacme.SchemeGroupVersion.WithResource("challenges"), + "status", gen.DefaultTestNamespace, gen.ChallengeFrom(baseChallenge, gen.SetChallengeProcessing(false), @@ -322,7 +327,8 @@ func TestSyncHappyPath(t *testing.T) { gen.SetChallengePresented(true), ), testIssuerHTTP01Enabled}, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction(cmacme.SchemeGroupVersion.WithResource("challenges"), + testpkg.NewAction(coretesting.NewUpdateSubresourceAction(cmacme.SchemeGroupVersion.WithResource("challenges"), + "status", gen.DefaultTestNamespace, gen.ChallengeFrom(baseChallenge, gen.SetChallengeProcessing(false), diff --git a/pkg/controller/acmeorders/sync.go b/pkg/controller/acmeorders/sync.go index 8cd68c82e..1390e0ccf 100644 --- a/pkg/controller/acmeorders/sync.go +++ b/pkg/controller/acmeorders/sync.go @@ -54,7 +54,7 @@ func (c *controller) Sync(ctx context.Context, o *cmacme.Order) (err error) { return } log.Info("updating Order resource status") - _, updateErr := c.cmClient.AcmeV1alpha2().Orders(o.Namespace).Update(o) + _, updateErr := c.cmClient.AcmeV1alpha2().Orders(o.Namespace).UpdateStatus(o) if err != nil { log.Error(err, "failed to update status") err = utilerrors.NewAggregate([]error{err, updateErr}) diff --git a/pkg/controller/acmeorders/sync_test.go b/pkg/controller/acmeorders/sync_test.go index 2a8cdd941..4bb7d91f7 100644 --- a/pkg/controller/acmeorders/sync_test.go +++ b/pkg/controller/acmeorders/sync_test.go @@ -153,7 +153,8 @@ dGVzdA== builder: &testpkg.Builder{ CertManagerObjects: []runtime.Object{testIssuerHTTP01TestCom, testOrder}, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction(cmacme.SchemeGroupVersion.WithResource("orders"), + testpkg.NewAction(coretesting.NewUpdateSubresourceAction(cmacme.SchemeGroupVersion.WithResource("orders"), + "status", testOrderPending.Namespace, gen.OrderFrom(testOrder, gen.SetOrderStatus(cmacme.OrderStatus{ State: cmacme.Pending, @@ -217,7 +218,8 @@ dGVzdA== builder: &testpkg.Builder{ CertManagerObjects: []runtime.Object{testIssuerHTTP01TestCom, testOrderPending, testAuthorizationChallengeValid}, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction(cmacme.SchemeGroupVersion.WithResource("orders"), + testpkg.NewAction(coretesting.NewUpdateSubresourceAction(cmacme.SchemeGroupVersion.WithResource("orders"), + "status", testOrderReady.Namespace, testOrderReady)), }, }, @@ -236,7 +238,8 @@ dGVzdA== builder: &testpkg.Builder{ CertManagerObjects: []runtime.Object{testIssuerHTTP01TestCom, testOrderReady, testAuthorizationChallengeValid}, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction(cmacme.SchemeGroupVersion.WithResource("orders"), + testpkg.NewAction(coretesting.NewUpdateSubresourceAction(cmacme.SchemeGroupVersion.WithResource("orders"), + "status", testOrderValid.Namespace, testOrderValid)), }, ExpectedEvents: []string{ @@ -262,7 +265,8 @@ dGVzdA== builder: &testpkg.Builder{ CertManagerObjects: []runtime.Object{testIssuerHTTP01TestCom, testOrderPending, testAuthorizationChallengeInvalid}, ExpectedActions: []testpkg.Action{ - testpkg.NewAction(coretesting.NewUpdateAction(cmacme.SchemeGroupVersion.WithResource("orders"), + testpkg.NewAction(coretesting.NewUpdateSubresourceAction(cmacme.SchemeGroupVersion.WithResource("orders"), + "status", testOrderInvalid.Namespace, testOrderInvalid)), }, }, From 9a807be80a8def43f80346a801b9b4f45ebab73b Mon Sep 17 00:00:00 2001 From: James Munnelly Date: Wed, 2 Oct 2019 13:25:37 +0100 Subject: [PATCH 5/6] Fix not returning updateErr if updating status failed Signed-off-by: James Munnelly --- pkg/controller/acmechallenges/sync.go | 2 +- pkg/controller/acmeorders/sync.go | 2 +- pkg/controller/certificates/BUILD.bazel | 1 + pkg/controller/certificates/sync.go | 20 +++++++------------- 4 files changed, 10 insertions(+), 15 deletions(-) diff --git a/pkg/controller/acmechallenges/sync.go b/pkg/controller/acmechallenges/sync.go index 24ddc4820..cf819a2f1 100644 --- a/pkg/controller/acmechallenges/sync.go +++ b/pkg/controller/acmechallenges/sync.go @@ -71,7 +71,7 @@ func (c *controller) Sync(ctx context.Context, ch *cmacme.Challenge) (err error) return } _, updateErr := c.cmClient.AcmeV1alpha2().Challenges(ch.Namespace).UpdateStatus(ch) - if err != nil { + if updateErr != nil { err = utilerrors.NewAggregate([]error{err, updateErr}) } }() diff --git a/pkg/controller/acmeorders/sync.go b/pkg/controller/acmeorders/sync.go index 1390e0ccf..4bb0b70a1 100644 --- a/pkg/controller/acmeorders/sync.go +++ b/pkg/controller/acmeorders/sync.go @@ -55,7 +55,7 @@ func (c *controller) Sync(ctx context.Context, o *cmacme.Order) (err error) { } log.Info("updating Order resource status") _, updateErr := c.cmClient.AcmeV1alpha2().Orders(o.Namespace).UpdateStatus(o) - if err != nil { + if updateErr != nil { log.Error(err, "failed to update status") err = utilerrors.NewAggregate([]error{err, updateErr}) return diff --git a/pkg/controller/certificates/BUILD.bazel b/pkg/controller/certificates/BUILD.bazel index 74d7f61fa..0e852b215 100644 --- a/pkg/controller/certificates/BUILD.bazel +++ b/pkg/controller/certificates/BUILD.bazel @@ -30,6 +30,7 @@ go_library( "@io_k8s_apimachinery//pkg/api/errors:go_default_library", "@io_k8s_apimachinery//pkg/apis/meta/v1:go_default_library", "@io_k8s_apimachinery//pkg/labels:go_default_library", + "@io_k8s_apimachinery//pkg/util/errors:go_default_library", "@io_k8s_client_go//kubernetes:go_default_library", "@io_k8s_client_go//listers/core/v1:go_default_library", "@io_k8s_client_go//tools/cache:go_default_library", diff --git a/pkg/controller/certificates/sync.go b/pkg/controller/certificates/sync.go index 55de11a06..eaf0e5cc1 100644 --- a/pkg/controller/certificates/sync.go +++ b/pkg/controller/certificates/sync.go @@ -28,6 +28,12 @@ import ( "time" "github.com/go-logr/logr" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" + utilerrors "k8s.io/apimachinery/pkg/util/errors" + apiutil "github.com/jetstack/cert-manager/pkg/api/util" cmapi "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1alpha2" cmmeta "github.com/jetstack/cert-manager/pkg/apis/meta/v1" @@ -37,10 +43,6 @@ import ( "github.com/jetstack/cert-manager/pkg/util/errors" "github.com/jetstack/cert-manager/pkg/util/kube" "github.com/jetstack/cert-manager/pkg/util/pki" - corev1 "k8s.io/api/core/v1" - apierrors "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/labels" ) func (c *certificateRequestManager) ProcessItem(ctx context.Context, key string) error { @@ -66,15 +68,7 @@ func (c *certificateRequestManager) ProcessItem(ctx context.Context, key string) err = c.processCertificate(ctx, updatedCert) log.V(logf.DebugLevel).Info("check if certificate status update is required") updateStatusErr := c.updateCertificateStatus(ctx, crt, updatedCert) - // TODO: combine errors - if err != nil { - return err - } - if updateStatusErr != nil { - return err - } - - return nil + return utilerrors.NewAggregate([]error{err, updateStatusErr}) } func (c *certificateRequestManager) updateCertificateStatus(ctx context.Context, old, crt *cmapi.Certificate) error { From e3b1e9bdb942f86fe0dd85aa09ce3d023287c29b Mon Sep 17 00:00:00 2001 From: James Munnelly Date: Wed, 2 Oct 2019 13:29:45 +0100 Subject: [PATCH 6/6] Call UpdateStatus before Update when finalizing Challenges Signed-off-by: James Munnelly --- pkg/controller/acmechallenges/sync.go | 29 +++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/pkg/controller/acmechallenges/sync.go b/pkg/controller/acmechallenges/sync.go index cf819a2f1..9add076ff 100644 --- a/pkg/controller/acmechallenges/sync.go +++ b/pkg/controller/acmechallenges/sync.go @@ -65,6 +65,10 @@ func (c *controller) Sync(ctx context.Context, ch *cmacme.Challenge) (err error) oldChal := ch ch = ch.DeepCopy() + if ch.DeletionTimestamp != nil { + return c.handleFinalizer(ctx, ch) + } + defer func() { // TODO: replace with more efficient comparison if reflect.DeepEqual(oldChal.Status, ch.Status) && len(oldChal.Finalizers) == len(ch.Finalizers) { @@ -76,10 +80,6 @@ func (c *controller) Sync(ctx context.Context, ch *cmacme.Challenge) (err error) } }() - if ch.DeletionTimestamp != nil { - return c.handleFinalizer(ctx, ch) - } - // bail out early on if processing=false, as this challenge has not been // scheduled yet. if ch.Status.Processing == false { @@ -233,7 +233,9 @@ func handleError(ch *cmacme.Challenge, err error) error { return err } -func (c *controller) handleFinalizer(ctx context.Context, ch *cmacme.Challenge) error { +// handleFinalizer will attempt to 'finalize' the Challenge resource by calling +// CleanUp if the resource is in a 'processing' state. +func (c *controller) handleFinalizer(ctx context.Context, ch *cmacme.Challenge) (err error) { log := logf.FromContext(ctx, "finalizer") if len(ch.Finalizers) == 0 { return nil @@ -242,7 +244,22 @@ func (c *controller) handleFinalizer(ctx context.Context, ch *cmacme.Challenge) log.V(logf.DebugLevel).Info("waiting to run challenge finalization...") return nil } - ch.Finalizers = ch.Finalizers[1:] + + defer func() { + // call UpdateStatus first as we may have updated the challenge.status.reason field + ch, updateErr := c.cmClient.AcmeV1alpha2().Challenges(ch.Namespace).UpdateStatus(ch) + if updateErr != nil { + err = utilerrors.NewAggregate([]error{err, updateErr}) + return + } + // call Update to remove the metadata.finalizers entry + ch.Finalizers = ch.Finalizers[1:] + _, updateErr = c.cmClient.AcmeV1alpha2().Challenges(ch.Namespace).Update(ch) + if updateErr != nil { + err = utilerrors.NewAggregate([]error{err, updateErr}) + return + } + }() if !ch.Status.Processing { return nil