diff --git a/pkg/apis/certmanager/v1alpha1/helpers.go b/pkg/apis/certmanager/v1alpha1/helpers.go new file mode 100644 index 000000000..c269ce861 --- /dev/null +++ b/pkg/apis/certmanager/v1alpha1/helpers.go @@ -0,0 +1,98 @@ +package v1alpha1 + +import ( + "fmt" + "time" + + "github.com/golang/glog" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func (i *IssuerStatus) ACMEStatus() *ACMEIssuerStatus { + if i.ACME == nil { + i.ACME = &ACMEIssuerStatus{} + } + return i.ACME +} + +func (a *ACMEIssuerDNS01Config) Provider(name string) (*ACMEIssuerDNS01Provider, error) { + for _, p := range a.Providers { + if p.Name == name { + return &(*&p), nil + } + } + return nil, fmt.Errorf("provider '%s' not found", name) +} + +func (a *ACMECertificateConfig) ConfigForDomain(domain string) ACMECertificateDomainConfig { + for _, cfg := range a.Config { + for _, d := range cfg.Domains { + if d == domain { + return cfg + } + } + } + return ACMECertificateDomainConfig{} +} + +func (c *CertificateStatus) ACMEStatus() *CertificateACMEStatus { + if c.ACME == nil { + c.ACME = &CertificateACMEStatus{} + } + return c.ACME +} + +func (c *CertificateACMEStatus) SaveAuthorization(a ACMEDomainAuthorization) { + for i, auth := range c.Authorizations { + if auth.Domain == a.Domain { + c.Authorizations[i] = a + return + } + } + c.Authorizations = append(c.Authorizations, a) +} + +func IssuerHasCondition(iss *Issuer, condition IssuerCondition) bool { + if len(iss.Status.Conditions) == 0 { + return false + } + for _, cond := range iss.Status.Conditions { + if condition.Type == cond.Type && condition.Status == cond.Status { + return true + } + } + return false +} + +func UpdateIssuerStatusCondition(iss *Issuer, conditionType IssuerConditionType, status ConditionStatus, reason, message string) *Issuer { + toUpdate := iss.DeepCopy() + newCondition := IssuerCondition{ + Type: conditionType, + Status: status, + Reason: reason, + Message: message, + } + + t := time.Now() + + if len(iss.Status.Conditions) == 0 { + glog.Infof("Setting lastTransitionTime for Issuer %q condition %q to %v", iss.Name, conditionType, t) + newCondition.LastTransitionTime = metav1.NewTime(t) + toUpdate.Status.Conditions = []IssuerCondition{newCondition} + } else { + for i, cond := range iss.Status.Conditions { + if cond.Type == conditionType { + if cond.Status != newCondition.Status { + glog.Infof("Found status change for Issuer %q condition %q: %q -> %q; setting lastTransitionTime to %v", iss.Name, conditionType, cond.Status, status, t) + newCondition.LastTransitionTime = metav1.NewTime(t) + } else { + newCondition.LastTransitionTime = cond.LastTransitionTime + } + + toUpdate.Status.Conditions[i] = newCondition + break + } + } + } + return toUpdate +} diff --git a/pkg/apis/certmanager/v1alpha1/types.go b/pkg/apis/certmanager/v1alpha1/types.go index 2d065855a..bfc9c497f 100644 --- a/pkg/apis/certmanager/v1alpha1/types.go +++ b/pkg/apis/certmanager/v1alpha1/types.go @@ -17,8 +17,6 @@ limitations under the License. package v1alpha1 import ( - "fmt" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -53,22 +51,58 @@ type IssuerSpec struct { // IssuerStatus contains status information about an Issuer type IssuerStatus struct { - Ready bool `json:"ready"` - ACME *ACMEIssuerStatus `json:"acme,omitempty"` + Conditions []IssuerCondition `json:"conditions"` + ACME *ACMEIssuerStatus `json:"acme,omitempty"` } -func (i *IssuerStatus) ACMEStatus() *ACMEIssuerStatus { - if i.ACME == nil { - i.ACME = &ACMEIssuerStatus{} - } - return i.ACME +// IssuerCondition contains condition information for an Issuer. +type IssuerCondition struct { + // Type of the condition, currently ('Ready'). + Type IssuerConditionType `json:"type"` + + // Status of the condition, one of ('True', 'False', 'Unknown'). + Status ConditionStatus `json:"status"` + + // LastTransitionTime is the timestamp corresponding to the last status + // change of this condition. + LastTransitionTime metav1.Time `json:"lastTransitionTime"` + + // Reason is a brief machine readable explanation for the condition's last + // transition. + Reason string `json:"reason"` + + // Message is a human readable description of the details of the last + // transition, complementing reason. + Message string `json:"message"` } -type ACMEIssuerStatus struct { - // URI is the unique account identifier, which can also be used to retrieve - // account details from the CA - URI string `json:"uri"` -} +// IssuerConditionType represents an Issuer condition value. +type IssuerConditionType string + +const ( + // IssuerConditionReady represents the fact that a given Issuer condition + // is in ready state. + IssuerConditionReady IssuerConditionType = "Ready" +) + +// ConditionStatus represents a condition's status. +type ConditionStatus string + +// These are valid condition statuses. "ConditionTrue" means a resource is in +// the condition; "ConditionFalse" means a resource is not in the condition; +// "ConditionUnknown" means kubernetes can't decide if a resource is in the +// condition or not. In the future, we could add other intermediate +// conditions, e.g. ConditionDegraded. +const ( + // ConditionTrue represents the fact that a given condition is true + ConditionTrue ConditionStatus = "True" + + // ConditionFalse represents the fact that a given condition is false + ConditionFalse ConditionStatus = "False" + + // ConditionUnknown represents the fact that a given condition is unknown + ConditionUnknown ConditionStatus = "Unknown" +) // ACMEIssuer contains the specification for an ACME issuer type ACMEIssuer struct { @@ -90,15 +124,6 @@ type ACMEIssuerDNS01Config struct { Providers []ACMEIssuerDNS01Provider `json:"providers"` } -func (a *ACMEIssuerDNS01Config) Provider(name string) (*ACMEIssuerDNS01Provider, error) { - for _, p := range a.Providers { - if p.Name == name { - return &(*&p), nil - } - } - return nil, fmt.Errorf("provider '%s' not found", name) -} - type ACMEIssuerDNS01Provider struct { Name string `json:"name"` @@ -130,6 +155,12 @@ type ACMEIssuerDNS01ProviderRoute53 struct { Region string `json:"region"` } +type ACMEIssuerStatus struct { + // URI is the unique account identifier, which can also be used to retrieve + // account details from the CA + URI string `json:"uri"` +} + // +genclient // +k8s:openapi-gen=true // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object @@ -172,17 +203,6 @@ type ACMECertificateConfig struct { Config []ACMECertificateDomainConfig `json:"config"` } -func (a *ACMECertificateConfig) ConfigForDomain(domain string) ACMECertificateDomainConfig { - for _, cfg := range a.Config { - for _, d := range cfg.Domains { - if d == domain { - return cfg - } - } - } - return ACMECertificateDomainConfig{} -} - type ACMECertificateDomainConfig struct { Domains []string `json:"domains"` HTTP01 *ACMECertificateHTTP01Config `json:"http-01,omitempty"` @@ -203,28 +223,11 @@ type CertificateStatus struct { ACME *CertificateACMEStatus `json:"acme,omitempty"` } -func (c *CertificateStatus) ACMEStatus() *CertificateACMEStatus { - if c.ACME == nil { - c.ACME = &CertificateACMEStatus{} - } - return c.ACME -} - // CertificateACMEStatus holds the status for an ACME issuer type CertificateACMEStatus struct { Authorizations []ACMEDomainAuthorization `json:"acme"` } -func (c *CertificateACMEStatus) SaveAuthorization(a ACMEDomainAuthorization) { - for i, auth := range c.Authorizations { - if auth.Domain == a.Domain { - c.Authorizations[i] = a - return - } - } - c.Authorizations = append(c.Authorizations, a) -} - // ACMEDomainAuthorization holds information about an ACME issuers domain // authorization type ACMEDomainAuthorization struct { diff --git a/pkg/apis/certmanager/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/certmanager/v1alpha1/zz_generated.deepcopy.go index c8cb5b363..ab29bc12d 100644 --- a/pkg/apis/certmanager/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/certmanager/v1alpha1/zz_generated.deepcopy.go @@ -108,6 +108,10 @@ func RegisterDeepCopies(scheme *runtime.Scheme) error { in.(*Issuer).DeepCopyInto(out.(*Issuer)) return nil }, InType: reflect.TypeOf(&Issuer{})}, + conversion.GeneratedDeepCopyFunc{Fn: func(in interface{}, out interface{}, c *conversion.Cloner) error { + in.(*IssuerCondition).DeepCopyInto(out.(*IssuerCondition)) + return nil + }, InType: reflect.TypeOf(&IssuerCondition{})}, conversion.GeneratedDeepCopyFunc{Fn: func(in interface{}, out interface{}, c *conversion.Cloner) error { in.(*IssuerList).DeepCopyInto(out.(*IssuerList)) return nil @@ -576,6 +580,23 @@ func (in *Issuer) DeepCopyObject() runtime.Object { } } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *IssuerCondition) DeepCopyInto(out *IssuerCondition) { + *out = *in + in.LastTransitionTime.DeepCopyInto(&out.LastTransitionTime) + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new IssuerCondition. +func (in *IssuerCondition) DeepCopy() *IssuerCondition { + if in == nil { + return nil + } + out := new(IssuerCondition) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *IssuerList) DeepCopyInto(out *IssuerList) { *out = *in @@ -638,6 +659,13 @@ func (in *IssuerSpec) DeepCopy() *IssuerSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *IssuerStatus) DeepCopyInto(out *IssuerStatus) { *out = *in + if in.Conditions != nil { + in, out := &in.Conditions, &out.Conditions + *out = make([]IssuerCondition, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } if in.ACME != nil { in, out := &in.ACME, &out.ACME if *in == nil { diff --git a/pkg/controller/certificates/sync.go b/pkg/controller/certificates/sync.go index 38d2bfd54..2e6851710 100644 --- a/pkg/controller/certificates/sync.go +++ b/pkg/controller/certificates/sync.go @@ -30,7 +30,12 @@ func (c *Controller) Sync(crt *v1alpha1.Certificate) (err error) { return fmt.Errorf("could not get issuer '%s' for certificate '%s': %s", crt.Spec.Issuer, crt.Name, err.Error()) } - if !issuerObj.Status.Ready { + issuerReady := !v1alpha1.IssuerHasCondition(issuerObj, v1alpha1.IssuerCondition{ + Type: v1alpha1.IssuerConditionReady, + Status: v1alpha1.ConditionTrue, + }) + + if !issuerReady { return fmt.Errorf("issuer '%s/%s' for certificate '%s' not ready", issuerObj.Namespace, issuerObj.Name, crt.Name) } diff --git a/pkg/issuer/acme/setup.go b/pkg/issuer/acme/setup.go index b80708ce9..d2a8819db 100644 --- a/pkg/issuer/acme/setup.go +++ b/pkg/issuer/acme/setup.go @@ -6,24 +6,33 @@ import ( "github.com/jetstack-experimental/cert-manager/pkg/apis/certmanager/v1alpha1" ) -func (a *Acme) Setup() (v1alpha1.IssuerStatus, error) { - updateStatus := a.issuer.Status.DeepCopy() +const ( + reasonAccountVerified = "ACME account verified" + reasonAccountRegistered = "ACME account registered" + reasonAccountRegistrationFailed = "ACME account registration failed" + messageAccountVerified = "The ACME account was verified with the ACME server" + messagedAccountRegistered = "The ACME account was registered with the ACME server" + messageAccountRegistrationFailed = "Failed to register ACME account with server: %s" +) + +func (a *Acme) Setup() (v1alpha1.IssuerStatus, error) { err := a.verifyAccount() if err == nil { - updateStatus.Ready = true - return *updateStatus, nil + update := v1alpha1.UpdateIssuerStatusCondition(a.issuer, v1alpha1.IssuerConditionReady, v1alpha1.ConditionTrue, reasonAccountVerified, reasonAccountRegistered) + return update.Status, nil } uri, err := a.registerAccount() if err != nil { - updateStatus.Ready = false - return *updateStatus, fmt.Errorf("error registering acme account: %s", err.Error()) + update := v1alpha1.UpdateIssuerStatusCondition(a.issuer, v1alpha1.IssuerConditionReady, v1alpha1.ConditionFalse, reasonAccountRegistrationFailed, fmt.Sprintf(messageAccountRegistrationFailed, err.Error())) + return update.Status, fmt.Errorf("error registering acme account: %s", err.Error()) } - updateStatus.ACMEStatus().URI = uri + update := v1alpha1.UpdateIssuerStatusCondition(a.issuer, v1alpha1.IssuerConditionReady, v1alpha1.ConditionFalse, reasonAccountRegistered, messageAccountVerified) + update.Status.ACMEStatus().URI = uri - return *updateStatus, nil + return update.Status, nil } diff --git a/test/e2e/framework/framework.go b/test/e2e/framework/framework.go index 332823fe7..0cd1c772f 100644 --- a/test/e2e/framework/framework.go +++ b/test/e2e/framework/framework.go @@ -22,6 +22,7 @@ import ( . "github.com/onsi/gomega" clientset "github.com/jetstack-experimental/cert-manager/pkg/client" + "github.com/jetstack-experimental/cert-manager/test/util" ) // Framework supports common operations used by e2e tests; it will keep a client & a namespace for you. @@ -106,6 +107,11 @@ func (f *Framework) AfterEach() { By("Deleting Certificate CustomResourceDefinition") err = DeleteCertificateCRD(f.APIExtensionsClientSet) Expect(err).NotTo(HaveOccurred()) + + err = util.WaitForCRDToNotExist(f.APIExtensionsClientSet.ApiextensionsV1beta1().CustomResourceDefinitions(), issuerCrd().Name) + Expect(err).NotTo(HaveOccurred()) + err = util.WaitForCRDToNotExist(f.APIExtensionsClientSet.ApiextensionsV1beta1().CustomResourceDefinitions(), certificateCrd().Name) + Expect(err).NotTo(HaveOccurred()) } // Wrapper function for ginkgo describe. Adds namespacing. diff --git a/test/e2e/issuer.go b/test/e2e/issuer.go index 21dbb5383..5610e7c05 100644 --- a/test/e2e/issuer.go +++ b/test/e2e/issuer.go @@ -48,12 +48,32 @@ var _ = framework.CertManagerDescribe("Issuer", func() { _, err := f.CertManagerClientSet.CertmanagerV1alpha1().Issuers(f.Namespace.Name).Create(newCertManagerACMEIssuer(issuerName, testingACMEURL, testingACMEEmail, testingACMEPrivateKey)) Expect(err).NotTo(HaveOccurred()) By("Waiting for Issuer to become Ready") - err = util.WaitForIssuerReady(f.CertManagerClientSet.CertmanagerV1alpha1().Issuers(f.Namespace.Name), issuerName) + err = util.WaitForIssuerCondition(f.CertManagerClientSet.CertmanagerV1alpha1().Issuers(f.Namespace.Name), + issuerName, + v1alpha1.IssuerCondition{ + Type: v1alpha1.IssuerConditionReady, + Status: v1alpha1.ConditionTrue, + }) + Expect(err).NotTo(HaveOccurred()) + }) + + It("should fail to register an ACME account", func() { + By("Creating an Issuer with an invalid server") + _, err := f.CertManagerClientSet.CertmanagerV1alpha1().Issuers(f.Namespace.Name).Create(newCertManagerACMEIssuer(issuerName, invalidACMEURL, testingACMEEmail, testingACMEPrivateKey)) + Expect(err).NotTo(HaveOccurred()) + By("Waiting for Issuer to become non-Ready") + err = util.WaitForIssuerCondition(f.CertManagerClientSet.CertmanagerV1alpha1().Issuers(f.Namespace.Name), + issuerName, + v1alpha1.IssuerCondition{ + Type: v1alpha1.IssuerConditionReady, + Status: v1alpha1.ConditionFalse, + }) Expect(err).NotTo(HaveOccurred()) }) }) const testingACMEURL = "https://acme-staging.api.letsencrypt.org/directory" +const invalidACMEURL = "http://not-a-real-acme-url.com" const testingACMEEmail = "test@example.com" const testingACMEPrivateKey = "test-acme-private-key" diff --git a/test/util/util.go b/test/util/util.go index e38dc6926..b3f0f02ed 100644 --- a/test/util/util.go +++ b/test/util/util.go @@ -4,21 +4,44 @@ import ( "fmt" "time" + "github.com/golang/glog" + apiextcs "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/typed/apiextensions/v1beta1" + "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/wait" + "github.com/jetstack-experimental/cert-manager/pkg/apis/certmanager/v1alpha1" clientset "github.com/jetstack-experimental/cert-manager/pkg/client/typed/certmanager/v1alpha1" ) -func WaitForIssuerReady(cl clientset.IssuerInterface, name string) error { +// WaitForIssuerCondition waits for the status of the named issuer to contain +// a condition whose type and status matches the supplied one. +func WaitForIssuerCondition(client clientset.IssuerInterface, name string, condition v1alpha1.IssuerCondition) error { return wait.PollImmediate(500*time.Millisecond, wait.ForeverTestTimeout, func() (bool, error) { - issuer, err := cl.Get(name, metav1.GetOptions{}) + glog.V(5).Infof("Waiting for issuer %v condition %#v", name, condition) + issuer, err := client.Get(name, metav1.GetOptions{}) if nil != err { - return false, fmt.Errorf("error getting Broker %v: %v", name, err) + return false, fmt.Errorf("error getting Issuer %v: %v", name, err) } - if issuer.Status.Ready { + return v1alpha1.IssuerHasCondition(issuer, condition), nil + }, + ) +} + +// WaitForCRDToNotExist waits for the CRD with the given name to no +// longer exist. +func WaitForCRDToNotExist(client apiextcs.CustomResourceDefinitionInterface, name string) error { + return wait.PollImmediate(500*time.Millisecond, wait.ForeverTestTimeout, + func() (bool, error) { + glog.V(5).Infof("Waiting for CRD %v to not exist", name) + _, err := client.Get(name, metav1.GetOptions{}) + if nil == err { + return false, nil + } + + if errors.IsNotFound(err) { return true, nil }