diff --git a/README.md b/README.md index a0b8bf3..58d787a 100644 --- a/README.md +++ b/README.md @@ -19,10 +19,166 @@ The ACK service controller for AWS Certificate Manager is free of charge. If you [samples]: https://github.com/aws-controllers-k8s/acmpca-controller/tree/main/samples ### Kubernetes Secrets -The ACK service controller for AWS Certificate Manager uses Kubernetes TLS Secrets to store the certificate chain and decrypted private key of the exported ACM certificate. Users are expected to create Secrets before creating Certificate resources. As these resources are created, the Secrets' `tls.crt` will be injected with the base64-encoded certificate and `tls.key` will be injected with the base64-encoded private key associated with the certificate. Users are responsible for deleting Secrets. +The ACK service controller for AWS Certificate Manager uses Kubernetes TLS Secrets in two ways: + +* **Export** — write an ACM-issued certificate and private key into a Secret (`exportTo`). +* **Import** — read an existing certificate and private key from a Secret and import them into ACM (`importFrom`, or opaque secret import via `certificate` / `privateKey`). + +For export, users are expected to create Secrets before creating Certificate resources. As these resources are created, the Secrets' `tls.crt` will be injected with the base64-encoded certificate and `tls.key` will be injected with the base64-encoded private key associated with the certificate. Users are responsible for deleting Secrets. In addition, after a certificate is successfully renewed by ACM, the ACK service controller for AWS Certificate Manager will automatically export the renewed certificate again so that the Kubernetes TLS Secret `exportTo` contains the certificate data and private key data of the renewed certificate. +For import, the Secret must already contain valid PEM data in `tls.crt` (certificate) and `tls.key` (private key). If `tls.crt` contains multiple PEM blocks (leaf certificate followed by intermediate certificates), the controller automatically splits them for the ACM `ImportCertificate` API. Secrets may be type `Opaque` or `kubernetes.io/tls`. + +#### Import Certificate + +There are two ways to import an existing certificate into ACM from Kubernetes Secrets: + +| Approach | Fields | Best for | +|----------|--------|----------| +| **TLS secret import** | `importFrom` | Standard TLS Secrets (`tls.crt` / `tls.key`) | +| **Opaque secret import** | `certificate`, `privateKey`, optional `certificateChain` | Custom secret keys or separate chain Secret | + +Both approaches call the ACM [ImportCertificate](https://docs.aws.amazon.com/acm/latest/userguide/import-certificate.html) API. Imported certificates cannot be used with certificate request fields such as `domainName`, or with `exportTo`. After import, the controller may populate fields such as `domainName`, `keyAlgorithm`, and `tags` in the resource spec from ACM. + +##### Import with `importFrom` + +To import from a standard TLS Secret, specify the Secret using the `importFrom` field. This field is **exclusive** with certificate request, export, and opaque secret import fields (`domainName`, `exportTo`, `certificate`, `privateKey`, etc.) and may be updated after creation. Set `certificateARN` with `importFrom` to replace an existing imported certificate. + +``` +apiVersion: v1 +kind: Secret +type: kubernetes.io/tls +metadata: + name: my-tls-secret + namespace: demo-app +data: + tls.crt: + tls.key: +--- +apiVersion: acm.services.k8s.aws/v1alpha1 +kind: Certificate +metadata: + name: imported-cert + namespace: demo-app +spec: + importFrom: + name: my-tls-secret +``` + +To reference a Secret in a different namespace: + +``` +spec: + importFrom: + name: my-tls-secret + namespace: other-namespace +``` + +To replace an existing imported certificate at a known ARN: + +``` +spec: + importFrom: + name: my-tls-secret + certificateARN: arn:aws:acm:region:account:certificate/12345678-1234-1234-1234-123456789012 +``` + +##### Opaque secret import + +Opaque secret import remains supported for backward compatibility. Set `certificate` to trigger import mode, and provide `privateKey` referencing the matching private key PEM. Each field is a `SecretKeyReference` with `name`, optional `namespace`, and `key` for the data entry within the Secret. + +Required fields: + +* **`certificate`** — secret reference to the leaf certificate PEM +* **`privateKey`** — secret reference to the private key PEM + +Optional fields: + +* **`certificateChain`** — secret reference to intermediate certificate PEMs (when not included in the certificate PEM) +* **`certificateARN`** — ARN of an existing imported certificate to replace +* **`tags`** — tags to apply to the imported certificate + +Opaque secret import is **exclusive** with `importFrom`, certificate request fields (`domainName`, `domainValidationOptions`, etc.), and `exportTo`. It cannot be combined with `importFrom`. + +If the certificate PEM contains multiple PEM blocks (leaf followed by intermediates), the controller automatically splits them for the ACM import API, even when `certificateChain` is not set. + +``` +apiVersion: v1 +kind: Secret +type: Opaque +metadata: + name: my-import-secret + namespace: demo-app +data: + tls.crt: + tls.key: +--- +apiVersion: acm.services.k8s.aws/v1alpha1 +kind: Certificate +metadata: + name: imported-cert-opaque + namespace: demo-app +spec: + certificate: + name: my-import-secret + key: tls.crt + privateKey: + name: my-import-secret + key: tls.key + tags: + - key: environment + value: dev +``` + +Certificate and chain in separate Secrets: + +``` +apiVersion: v1 +kind: Secret +type: Opaque +metadata: + name: my-cert + namespace: demo-app +data: + cert.pem: +--- +apiVersion: v1 +kind: Secret +type: Opaque +metadata: + name: my-key + namespace: demo-app +data: + key.pem: +--- +apiVersion: v1 +kind: Secret +type: Opaque +metadata: + name: my-chain + namespace: demo-app +data: + chain.pem: +--- +apiVersion: acm.services.k8s.aws/v1alpha1 +kind: Certificate +metadata: + name: imported-cert-opaque + namespace: demo-app +spec: + certificate: + name: my-cert + key: cert.pem + privateKey: + name: my-key + key: key.pem + certificateChain: + name: my-chain + key: chain.pem +``` + +**Note:** Opaque secret import fields (`certificate`, `privateKey`, `certificateChain`) are immutable once set. For new deployments, prefer `importFrom` when importing from a standard TLS Secret. #### Export Certificate To export an ACM certificate to a Kubernetes TLS Secret, users must specify the namespace and the name of the Secret using the `exportTo` field of the Certificate resource, as shown below. diff --git a/apis/v1alpha1/certificate.go b/apis/v1alpha1/certificate.go index ec2057c..0cbc8bc 100644 --- a/apis/v1alpha1/certificate.go +++ b/apis/v1alpha1/certificate.go @@ -21,14 +21,17 @@ import ( ) // CertificateSpec defines the desired state of Certificate. +// +kubebuilder:validation:XValidation:rule="!has(self.importFrom) || (!has(self.certificate) && !has(self.certificateAuthorityARN) && !has(self.certificateAuthorityRef) && !has(self.certificateChain) && !has(self.domainName) && !has(self.domainValidationOptions) && !has(self.exportTo) && !has(self.keyAlgorithm) && !has(self.options) && !has(self.privateKey) && !has(self.subjectAlternativeNames))",message="importFrom cannot be set with certificate request, export, or opaque secret import fields" type CertificateSpec struct { - // The Certificate to import into AWS Certificate Manager (ACM) to use with services that are integrated with ACM. - // This field is only valid when importing an existing certificate into ACM. + // Opaque secret import field. Reference to a Kubernetes Secret key containing the leaf certificate PEM to + // import into ACM. Requires privateKey. Mutually exclusive with importFrom and certificate request + // fields. If the PEM contains multiple certificate blocks, the controller splits leaf and + // intermediate certificates automatically. Immutable once set. // +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Value is immutable once set" Certificate *ackv1alpha1.SecretKeyReference `json:"certificate,omitempty"` - // The Amazon Resource Name (ARN) of an imported certificate to replace. This field is only valid when importing - // an existing certificate into ACM. + // The Amazon Resource Name (ARN) of an imported certificate to replace. Valid with opaque secret + // import (certificate/privateKey) or importFrom. Immutable once set. // +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Value is immutable once set" CertificateARN *string `json:"certificateARN,omitempty"` // The Amazon Resource Name (ARN) of the private certificate authority (CA) @@ -44,6 +47,9 @@ type CertificateSpec struct { // +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Value is immutable once set" CertificateAuthorityARN *string `json:"certificateAuthorityARN,omitempty"` CertificateAuthorityRef *ackv1alpha1.AWSResourceReferenceWrapper `json:"certificateAuthorityRef,omitempty"` + // Opaque secret import field. Optional reference to a Kubernetes Secret key containing intermediate + // certificate PEMs. Use when the chain is stored separately from certificate. Mutually exclusive + // with importFrom and certificate request fields. Immutable once set. // +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Value is immutable once set" CertificateChain *ackv1alpha1.SecretKeyReference `json:"certificateChain,omitempty"` // Fully qualified domain name (FQDN), such as www.example.com, that you want @@ -66,6 +72,13 @@ type CertificateSpec struct { DomainValidationOptions []*DomainValidationOption `json:"domainValidationOptions,omitempty"` // +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Value is immutable once set" ExportTo *ackv1alpha1.SecretKeyReference `json:"exportTo,omitempty"` + // Reference to an existing Kubernetes TLS Secret to import into ACM. The certificate PEM is read from + // tls.crt and the private key is read from tls.key in the Secret. If tls.crt contains multiple PEM + // blocks (leaf certificate followed by intermediate certificates), they are automatically split for + // ACM import. Mutually exclusive with opaque secret import fields (certificate, privateKey, + // certificateChain) and certificate request fields. May be updated after creation. certificateARN + // may be set to replace an existing imported certificate. + ImportFrom *ackv1alpha1.SecretReference `json:"importFrom,omitempty"` // Specifies the algorithm of the public and private key pair that your certificate // uses to encrypt data. RSA is the default key algorithm for ACM certificates. // Elliptic Curve Digital Signature Algorithm (ECDSA) keys are smaller, offering @@ -108,8 +121,9 @@ type CertificateSpec struct { // as well as outside the Amazon Web Services Cloud. For more information, see // Certificate Manager exportable public certificate (https://docs.aws.amazon.com/acm/latest/userguide/acm-exportable-certificates.html). Options *CertificateOptions `json:"options,omitempty"` - // The private key that matches the public key in the certificate. This field is only valid when importing - // an existing certificate into ACM. + // Opaque secret import field. Reference to a Kubernetes Secret key containing the private key PEM that + // matches certificate. Required when certificate is set. Mutually exclusive with importFrom and + // certificate request fields. Immutable once set. // +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Value is immutable once set" PrivateKey *ackv1alpha1.SecretKeyReference `json:"privateKey,omitempty"` // Additional FQDNs to be included in the Subject Alternative Name extension diff --git a/apis/v1alpha1/generator.yaml b/apis/v1alpha1/generator.yaml index 2d42052..6931d45 100644 --- a/apis/v1alpha1/generator.yaml +++ b/apis/v1alpha1/generator.yaml @@ -45,6 +45,9 @@ operations: operation_type: DELETE resources: Certificate: + spec_validations: + - rule: "!has(self.importFrom) || (!has(self.certificate) && !has(self.certificateAuthorityARN) && !has(self.certificateAuthorityRef) && !has(self.certificateChain) && !has(self.domainName) && !has(self.domainValidationOptions) && !has(self.exportTo) && !has(self.keyAlgorithm) && !has(self.options) && !has(self.privateKey) && !has(self.subjectAlternativeNames))" + message: "importFrom cannot be set with certificate request, export, or opaque secret import fields" hooks: delta_pre_compare: template_path: hooks/certificate/delta_pre_compare.go.tpl @@ -62,6 +65,8 @@ resources: template_path: hooks/certificate/sdk_file_end.go.tpl late_initialize_post_read_one: template_path: hooks/certificate/late_initialize_post_read_one.go.tpl + sdk_read_one_post_set_output: + template_path: hooks/certificate/sdk_read_one_post_set_output.go.tpl exceptions: errors: 404: @@ -82,10 +87,17 @@ resources: is_secret: true compare: is_ignored: true + ImportFrom: + type: "bytes" + is_secret_reference: true + compare: + is_ignored: true DomainName: is_primary_key: false is_required: false is_immutable: true + compare: + is_ignored: true Certificate: type: "bytes" is_secret: true diff --git a/apis/v1alpha1/zz_generated.deepcopy.go b/apis/v1alpha1/zz_generated.deepcopy.go index 597fe7d..19cf367 100644 --- a/apis/v1alpha1/zz_generated.deepcopy.go +++ b/apis/v1alpha1/zz_generated.deepcopy.go @@ -1293,6 +1293,11 @@ func (in *CertificateSpec) DeepCopyInto(out *CertificateSpec) { *out = new(corev1alpha1.SecretKeyReference) **out = **in } + if in.ImportFrom != nil { + in, out := &in.ImportFrom, &out.ImportFrom + *out = new(corev1alpha1.SecretReference) + **out = **in + } if in.KeyAlgorithm != nil { in, out := &in.KeyAlgorithm, &out.KeyAlgorithm *out = new(string) diff --git a/config/crd/bases/acm.services.k8s.aws_certificates.yaml b/config/crd/bases/acm.services.k8s.aws_certificates.yaml index afb98c8..69b0781 100644 --- a/config/crd/bases/acm.services.k8s.aws_certificates.yaml +++ b/config/crd/bases/acm.services.k8s.aws_certificates.yaml @@ -41,8 +41,10 @@ spec: properties: certificate: description: |- - The Certificate to import into AWS Certificate Manager (ACM) to use with services that are integrated with ACM. - This field is only valid when importing an existing certificate into ACM. + Opaque secret import field. Reference to a Kubernetes Secret key containing the leaf certificate PEM to + import into ACM. Requires privateKey. Mutually exclusive with importFrom and certificate request + fields. If the PEM contains multiple certificate blocks, the controller splits leaf and + intermediate certificates automatically. Immutable once set. properties: key: description: Key is the key within the secret @@ -64,8 +66,8 @@ spec: rule: self == oldSelf certificateARN: description: |- - The Amazon Resource Name (ARN) of an imported certificate to replace. This field is only valid when importing - an existing certificate into ACM. + The Amazon Resource Name (ARN) of an imported certificate to replace. Valid with opaque secret + import (certificate/privateKey) or importFrom. Immutable once set. type: string x-kubernetes-validations: - message: Value is immutable once set @@ -105,8 +107,9 @@ spec: type: object certificateChain: description: |- - SecretKeyReference combines a k8s corev1.SecretReference with a - specific key within the referred-to Secret + Opaque secret import field. Optional reference to a Kubernetes Secret key containing intermediate + certificate PEMs. Use when the chain is stored separately from certificate. Mutually exclusive + with importFrom and certificate request fields. Immutable once set. properties: key: description: Key is the key within the secret @@ -185,6 +188,25 @@ spec: x-kubernetes-validations: - message: Value is immutable once set rule: self == oldSelf + importFrom: + description: |- + Reference to an existing Kubernetes TLS Secret to import into ACM. The certificate PEM is read from + tls.crt and the private key is read from tls.key in the Secret. If tls.crt contains multiple PEM + blocks (leaf certificate followed by intermediate certificates), they are automatically split for + ACM import. Mutually exclusive with opaque secret import fields (certificate, privateKey, + certificateChain) and certificate request fields. May be updated after creation. certificateARN + may be set to replace an existing imported certificate. + properties: + name: + description: name is unique within a namespace to reference a + secret resource. + type: string + namespace: + description: namespace defines the space within which the secret + name must be unique. + type: string + type: object + x-kubernetes-map-type: atomic keyAlgorithm: description: |- Specifies the algorithm of the public and private key pair that your certificate @@ -244,8 +266,9 @@ spec: type: object privateKey: description: |- - The private key that matches the public key in the certificate. This field is only valid when importing - an existing certificate into ACM. + Opaque secret import field. Reference to a Kubernetes Secret key containing the private key PEM that + matches certificate. Required when certificate is set. Mutually exclusive with importFrom and + certificate request fields. Immutable once set. properties: key: description: Key is the key within the secret @@ -308,6 +331,14 @@ spec: type: object type: array type: object + x-kubernetes-validations: + - message: importFrom cannot be set with certificate request, export, + or opaque secret import fields + rule: '!has(self.importFrom) || (!has(self.certificate) && !has(self.certificateAuthorityARN) + && !has(self.certificateAuthorityRef) && !has(self.certificateChain) + && !has(self.domainName) && !has(self.domainValidationOptions) && + !has(self.exportTo) && !has(self.keyAlgorithm) && !has(self.options) + && !has(self.privateKey) && !has(self.subjectAlternativeNames))' status: description: CertificateStatus defines the observed state of Certificate properties: diff --git a/config/iam/recommended-inline-policy b/config/iam/recommended-inline-policy index 8612414..4591d7c 100644 --- a/config/iam/recommended-inline-policy +++ b/config/iam/recommended-inline-policy @@ -6,6 +6,7 @@ "Effect": "Allow", "Action": [ "acm:DescribeCertificate", + "acm:GetCertificate", "acm:ImportCertificate", "acm:RequestCertificate", "acm:UpdateCertificateOptions", diff --git a/documentation.yaml b/documentation.yaml index 9b090b3..d93fcc9 100644 --- a/documentation.yaml +++ b/documentation.yaml @@ -3,16 +3,32 @@ resources: fields: Certificate: prepend: | - The Certificate to import into AWS Certificate Manager (ACM) to use with services that are integrated with ACM. - This field is only valid when importing an existing certificate into ACM. + Opaque secret import field. Reference to a Kubernetes Secret key containing the leaf certificate PEM to + import into ACM. Requires privateKey. Mutually exclusive with importFrom and certificate request + fields. If the PEM contains multiple certificate blocks, the controller splits leaf and + intermediate certificates automatically. Immutable once set. PrivateKey: prepend: | - The private key that matches the public key in the certificate. This field is only valid when importing - an existing certificate into ACM. + Opaque secret import field. Reference to a Kubernetes Secret key containing the private key PEM that + matches certificate. Required when certificate is set. Mutually exclusive with importFrom and + certificate request fields. Immutable once set. + CertificateChain: + prepend: | + Opaque secret import field. Optional reference to a Kubernetes Secret key containing intermediate + certificate PEMs. Use when the chain is stored separately from certificate. Mutually exclusive + with importFrom and certificate request fields. Immutable once set. CertificateARN: prepend: | - The Amazon Resource Name (ARN) of an imported certificate to replace. This field is only valid when importing - an existing certificate into ACM. + The Amazon Resource Name (ARN) of an imported certificate to replace. Valid with opaque secret + import (certificate/privateKey) or importFrom. Immutable once set. + ImportFrom: + prepend: | + Reference to an existing Kubernetes TLS Secret to import into ACM. The certificate PEM is read from + tls.crt and the private key is read from tls.key in the Secret. If tls.crt contains multiple PEM + blocks (leaf certificate followed by intermediate certificates), they are automatically split for + ACM import. Mutually exclusive with opaque secret import fields (certificate, privateKey, + certificateChain) and certificate request fields. May be updated after creation. certificateARN + may be set to replace an existing imported certificate. AcmeEndpoint: fields: diff --git a/generator.yaml b/generator.yaml index 2d42052..6931d45 100644 --- a/generator.yaml +++ b/generator.yaml @@ -45,6 +45,9 @@ operations: operation_type: DELETE resources: Certificate: + spec_validations: + - rule: "!has(self.importFrom) || (!has(self.certificate) && !has(self.certificateAuthorityARN) && !has(self.certificateAuthorityRef) && !has(self.certificateChain) && !has(self.domainName) && !has(self.domainValidationOptions) && !has(self.exportTo) && !has(self.keyAlgorithm) && !has(self.options) && !has(self.privateKey) && !has(self.subjectAlternativeNames))" + message: "importFrom cannot be set with certificate request, export, or opaque secret import fields" hooks: delta_pre_compare: template_path: hooks/certificate/delta_pre_compare.go.tpl @@ -62,6 +65,8 @@ resources: template_path: hooks/certificate/sdk_file_end.go.tpl late_initialize_post_read_one: template_path: hooks/certificate/late_initialize_post_read_one.go.tpl + sdk_read_one_post_set_output: + template_path: hooks/certificate/sdk_read_one_post_set_output.go.tpl exceptions: errors: 404: @@ -82,10 +87,17 @@ resources: is_secret: true compare: is_ignored: true + ImportFrom: + type: "bytes" + is_secret_reference: true + compare: + is_ignored: true DomainName: is_primary_key: false is_required: false is_immutable: true + compare: + is_ignored: true Certificate: type: "bytes" is_secret: true diff --git a/helm/crds/acm.services.k8s.aws_certificates.yaml b/helm/crds/acm.services.k8s.aws_certificates.yaml index 6facc64..862f1e7 100644 --- a/helm/crds/acm.services.k8s.aws_certificates.yaml +++ b/helm/crds/acm.services.k8s.aws_certificates.yaml @@ -41,8 +41,10 @@ spec: properties: certificate: description: |- - The Certificate to import into AWS Certificate Manager (ACM) to use with services that are integrated with ACM. - This field is only valid when importing an existing certificate into ACM. + Opaque secret import field. Reference to a Kubernetes Secret key containing the leaf certificate PEM to + import into ACM. Requires privateKey. Mutually exclusive with importFrom and certificate request + fields. If the PEM contains multiple certificate blocks, the controller splits leaf and + intermediate certificates automatically. Immutable once set. properties: key: description: Key is the key within the secret @@ -64,8 +66,8 @@ spec: rule: self == oldSelf certificateARN: description: |- - The Amazon Resource Name (ARN) of an imported certificate to replace. This field is only valid when importing - an existing certificate into ACM. + The Amazon Resource Name (ARN) of an imported certificate to replace. Valid with opaque secret + import (certificate/privateKey) or importFrom. Immutable once set. type: string x-kubernetes-validations: - message: Value is immutable once set @@ -105,8 +107,9 @@ spec: type: object certificateChain: description: |- - SecretKeyReference combines a k8s corev1.SecretReference with a - specific key within the referred-to Secret + Opaque secret import field. Optional reference to a Kubernetes Secret key containing intermediate + certificate PEMs. Use when the chain is stored separately from certificate. Mutually exclusive + with importFrom and certificate request fields. Immutable once set. properties: key: description: Key is the key within the secret @@ -185,6 +188,25 @@ spec: x-kubernetes-validations: - message: Value is immutable once set rule: self == oldSelf + importFrom: + description: |- + Reference to an existing Kubernetes TLS Secret to import into ACM. The certificate PEM is read from + tls.crt and the private key is read from tls.key in the Secret. If tls.crt contains multiple PEM + blocks (leaf certificate followed by intermediate certificates), they are automatically split for + ACM import. Mutually exclusive with opaque secret import fields (certificate, privateKey, + certificateChain) and certificate request fields. May be updated after creation. certificateARN + may be set to replace an existing imported certificate. + properties: + name: + description: name is unique within a namespace to reference a + secret resource. + type: string + namespace: + description: namespace defines the space within which the secret + name must be unique. + type: string + type: object + x-kubernetes-map-type: atomic keyAlgorithm: description: |- Specifies the algorithm of the public and private key pair that your certificate @@ -244,8 +266,9 @@ spec: type: object privateKey: description: |- - The private key that matches the public key in the certificate. This field is only valid when importing - an existing certificate into ACM. + Opaque secret import field. Reference to a Kubernetes Secret key containing the private key PEM that + matches certificate. Required when certificate is set. Mutually exclusive with importFrom and + certificate request fields. Immutable once set. properties: key: description: Key is the key within the secret @@ -308,6 +331,14 @@ spec: type: object type: array type: object + x-kubernetes-validations: + - message: importFrom cannot be set with certificate request, export, + or opaque secret import fields + rule: '!has(self.importFrom) || (!has(self.certificate) && !has(self.certificateAuthorityARN) + && !has(self.certificateAuthorityRef) && !has(self.certificateChain) + && !has(self.domainName) && !has(self.domainValidationOptions) && + !has(self.exportTo) && !has(self.keyAlgorithm) && !has(self.options) + && !has(self.privateKey) && !has(self.subjectAlternativeNames))' status: description: CertificateStatus defines the observed state of Certificate properties: diff --git a/olm/olmconfig.yaml b/olm/olmconfig.yaml index 2b9abc0..da8b552 100644 --- a/olm/olmconfig.yaml +++ b/olm/olmconfig.yaml @@ -31,6 +31,10 @@ description: |- samples: - kind: Certificate spec: '{}' +- kind: Certificate + spec: | + importFrom: + name: my-tls-secret maintainers: - name: "acm maintainer team" email: "ack-maintainers@amazon.com" diff --git a/pkg/resource/certificate/delta.go b/pkg/resource/certificate/delta.go index b0ef75e..4d93767 100644 --- a/pkg/resource/certificate/delta.go +++ b/pkg/resource/certificate/delta.go @@ -43,6 +43,7 @@ func newResourceDelta( } compareCertificateIssuedAt(delta, a, b) compareKeyAlgorithm(delta, a, b) + compareDomainName(delta, a, b) if ackcompare.HasNilDifference(a.ko.Spec.CertificateARN, b.ko.Spec.CertificateARN) { delta.Add("Spec.CertificateARN", a.ko.Spec.CertificateARN, b.ko.Spec.CertificateARN) @@ -61,13 +62,6 @@ func newResourceDelta( if !equality.Semantic.Equalities.DeepEqual(a.ko.Spec.CertificateAuthorityRef, b.ko.Spec.CertificateAuthorityRef) { delta.Add("Spec.CertificateAuthorityRef", a.ko.Spec.CertificateAuthorityRef, b.ko.Spec.CertificateAuthorityRef) } - if ackcompare.HasNilDifference(a.ko.Spec.DomainName, b.ko.Spec.DomainName) { - delta.Add("Spec.DomainName", a.ko.Spec.DomainName, b.ko.Spec.DomainName) - } else if a.ko.Spec.DomainName != nil && b.ko.Spec.DomainName != nil { - if *a.ko.Spec.DomainName != *b.ko.Spec.DomainName { - delta.Add("Spec.DomainName", a.ko.Spec.DomainName, b.ko.Spec.DomainName) - } - } if ackcompare.HasNilDifference(a.ko.Spec.Options, b.ko.Spec.Options) { delta.Add("Spec.Options", a.ko.Spec.Options, b.ko.Spec.Options) } else if a.ko.Spec.Options != nil && b.ko.Spec.Options != nil { diff --git a/pkg/resource/certificate/hooks.go b/pkg/resource/certificate/hooks.go index d42cc30..17ed3a5 100644 --- a/pkg/resource/certificate/hooks.go +++ b/pkg/resource/certificate/hooks.go @@ -14,6 +14,7 @@ package certificate import ( + "bytes" "context" "crypto/ecdsa" "crypto/rand" @@ -24,19 +25,29 @@ import ( "fmt" "io" "strings" + "time" "github.com/aws-controllers-k8s/acm-controller/pkg/tags" ackv1alpha1 "github.com/aws-controllers-k8s/runtime/apis/core/v1alpha1" ackcompare "github.com/aws-controllers-k8s/runtime/pkg/compare" ackerr "github.com/aws-controllers-k8s/runtime/pkg/errors" + ackrequeue "github.com/aws-controllers-k8s/runtime/pkg/requeue" ackrtlog "github.com/aws-controllers-k8s/runtime/pkg/runtime/log" svcsdk "github.com/aws/aws-sdk-go-v2/service/acm" pkcs8 "github.com/youmark/pkcs8" + + svcapitypes "github.com/aws-controllers-k8s/acm-controller/apis/v1alpha1" ) const ( // DNS validation only works for up to 5 chained CNAME records limitDomainValidationOptionsPublic = 5 + // tlsCertificateSecretDataKey is the secret data key for the certificate when + // importing from a Kubernetes TLS Secret via importFrom. + tlsCertificateSecretDataKey = "tls.crt" + // tlsPrivateKeySecretDataKey is the secret data key for the private key when + // importing from a Kubernetes TLS Secret via importFrom. + tlsPrivateKeySecretDataKey = "tls.key" ) var ( @@ -72,29 +83,140 @@ func validatePublicValidationOptions( return nil } -// maybeImportCertificate imports a certificate into ACM if Spec.Certificate is set. -func (rm *resourceManager) maybeImportCertificate(ctx context.Context, r *resource) (*resource, bool, error) { +// importSecretRefs holds the resolved secret references for certificate import. +type importSecretRefs struct { + Certificate *ackv1alpha1.SecretKeyReference + PrivateKey *ackv1alpha1.SecretKeyReference + CertificateChain *ackv1alpha1.SecretKeyReference +} + +func isImportCertificateSpec(certSpec svcapitypes.CertificateSpec) bool { + return certSpec.Certificate != nil || certSpec.ImportFrom != nil +} + +func importSecretRefsFromSpec(certSpec svcapitypes.CertificateSpec) (*importSecretRefs, error) { + if certSpec.ImportFrom != nil { + certRef := ackv1alpha1.SecretKeyReference{ + SecretReference: certSpec.ImportFrom.SecretReference, + Key: tlsCertificateSecretDataKey, + } + keyRef := ackv1alpha1.SecretKeyReference{ + SecretReference: certSpec.ImportFrom.SecretReference, + Key: tlsPrivateKeySecretDataKey, + } + return &importSecretRefs{ + Certificate: &certRef, + PrivateKey: &keyRef, + }, nil + } + return &importSecretRefs{ + Certificate: certSpec.Certificate, + PrivateKey: certSpec.PrivateKey, + CertificateChain: certSpec.CertificateChain, + }, nil +} + +func (rm *resourceManager) secretValueFromReference( + ctx context.Context, + ref *ackv1alpha1.SecretKeyReference, +) (string, error) { + if ref == nil { + return "", nil + } + value, err := rm.rr.SecretValueFromReference(ctx, ref) + if err != nil { + return "", ackrequeue.Needed(err) + } + return value, nil +} + +func validateImportFromExclusivity(certSpec svcapitypes.CertificateSpec) error { + if certSpec.ImportFrom == nil { + return nil + } + // Tags are excluded because the controller injects default tags via + // EnsureTags before create. Request fields populated from ACM are cleared + // before this validation when importFrom manages an existing certificate. + if certSpec.Certificate != nil || certSpec.PrivateKey != nil || certSpec.CertificateChain != nil || + certSpec.ExportTo != nil || + certSpec.CertificateAuthorityARN != nil || certSpec.CertificateAuthorityRef != nil || + certSpec.DomainName != nil || len(certSpec.DomainValidationOptions) > 0 || + certSpec.KeyAlgorithm != nil || certSpec.Options != nil || + len(certSpec.SubjectAlternativeNames) > 0 { + return ackerr.NewTerminalError(errors.New("importFrom cannot be set with certificate request, export, or opaque secret import fields")) + } + return nil +} + +func clearImportFromObservedSpecFields(certSpec *svcapitypes.CertificateSpec) { + certSpec.DomainValidationOptions = nil + certSpec.KeyAlgorithm = nil + certSpec.SubjectAlternativeNames = nil + certSpec.Options = nil + certSpec.DomainName = nil +} + +// splitCertificateAndChain splits PEM data that may contain a leaf certificate +// followed by intermediate certificates (as stored in a Kubernetes TLS Secret's +// tls.crt key) into separate certificate and chain byte slices for ACM import. +func splitCertificateAndChain(pemData []byte) (certificate []byte, chain []byte, err error) { + var certBlocks [][]byte + remaining := pemData + for { + var block *pem.Block + block, remaining = pem.Decode(remaining) + if block == nil { + break + } + if block.Type != "CERTIFICATE" { + continue + } + certBlocks = append(certBlocks, pem.EncodeToMemory(block)) + } + if len(certBlocks) == 0 { + return nil, nil, errors.New("no certificate found in PEM data") + } + certificate = certBlocks[0] + if len(certBlocks) > 1 { + chain = bytes.Join(certBlocks[1:], nil) + } + return certificate, chain, nil +} + +// finalizeImportCertificateInput applies ImportCertificate request constraints that +// are not expressed in the CRD schema. ACM does not permit tags when re-importing +// a certificate at an existing ARN. +func finalizeImportCertificateInput(input *svcsdk.ImportCertificateInput) { + if input.CertificateArn != nil && *input.CertificateArn != "" { + input.Tags = nil + } +} + +// ImportTlsCertificate imports a certificate into ACM if Spec.Certificate or +// Spec.ImportFrom is set. +func (rm *resourceManager) ImportTlsCertificate(ctx context.Context, r *resource) (*resource, bool, error) { certSpec := r.ko.Spec - if certSpec.Certificate != nil { + if isImportCertificateSpec(certSpec) { // When re-importing a certificate that was previously created (has an // ARN), clear request-only fields that were populated by late // initialization / DescribeCertificate. These are informational for // imported certs and will be re-populated after the new import succeeds. if r.ko.Status.ACKResourceMetadata != nil && r.ko.Status.ACKResourceMetadata.ARN != nil { - certSpec.DomainValidationOptions = nil - certSpec.KeyAlgorithm = nil - certSpec.SubjectAlternativeNames = nil - certSpec.Options = nil - certSpec.DomainName = nil + clearImportFromObservedSpecFields(&certSpec) } - if certSpec.DomainName != nil || len(certSpec.DomainValidationOptions) > 0 || certSpec.KeyAlgorithm != nil || - len(certSpec.SubjectAlternativeNames) > 0 || certSpec.Options != nil { + if err := validateImportFromExclusivity(certSpec); err != nil { + return nil, false, err + } + if certSpec.ImportFrom == nil && (certSpec.DomainName != nil || len(certSpec.DomainValidationOptions) > 0 || certSpec.KeyAlgorithm != nil || + len(certSpec.SubjectAlternativeNames) > 0 || certSpec.Options != nil) { return nil, false, ackerr.NewTerminalError(errors.New("cannot set fields used for requesting a certificate when importing a certificate")) } input, err := rm.newImportCertificateInput(ctx, r) if err != nil { return nil, false, err } + setImportCertificateARN(input, r) + finalizeImportCertificateInput(input) if len(input.PrivateKey) == 0 { return nil, false, ackerr.NewTerminalError(errors.New("privateKey is required when importing a certificate")) } @@ -104,7 +226,7 @@ func (rm *resourceManager) maybeImportCertificate(ctx context.Context, r *resour } return created, true, nil } - if certSpec.DomainName != nil && (certSpec.Certificate != nil || certSpec.PrivateKey != nil || certSpec.CertificateChain != nil) { + if certSpec.DomainName != nil && (certSpec.Certificate != nil || certSpec.PrivateKey != nil || certSpec.CertificateChain != nil || certSpec.ImportFrom != nil) { return nil, false, ackerr.NewTerminalError(errors.New("cannot set fields used for importing a certificate when requesting a certificate")) } return nil, false, nil @@ -139,6 +261,157 @@ func (rm *resourceManager) importCertificate( return created, nil } +func isImportFromManagedCertificate(r *resource) bool { + if r == nil || r.ko == nil || r.ko.Spec.ImportFrom == nil { + return false + } + if r.ko.Status.ACKResourceMetadata == nil || r.ko.Status.ACKResourceMetadata.ARN == nil { + return false + } + return r.ko.Status.Type != nil && + *r.ko.Status.Type == string(svcapitypes.CertificateType_IMPORTED) +} + +func parseLeafCertificate(pemData []byte) (*x509.Certificate, error) { + certPEM, _, err := splitCertificateAndChain(pemData) + if err != nil { + return nil, err + } + block, _ := pem.Decode(certPEM) + if block == nil { + return nil, errors.New("failed to decode certificate PEM") + } + return x509.ParseCertificate(block.Bytes) +} + +func (rm *resourceManager) importFromSecretLeafCertificate( + ctx context.Context, + certSpec svcapitypes.CertificateSpec, +) (*x509.Certificate, error) { + refs, err := importSecretRefsFromSpec(certSpec) + if err != nil { + return nil, err + } + if refs.Certificate == nil { + return nil, errors.New("importFrom secret reference is missing") + } + + pemData, err := rm.secretValueFromReference(ctx, refs.Certificate) + if err != nil { + return nil, err + } + if pemData == "" { + return nil, errors.New("importFrom TLS secret certificate is empty") + } + return parseLeafCertificate([]byte(pemData)) +} + +// certificatesMatch compares the complete DER-encoded leaf certificates. +func certificatesMatch(a, b *x509.Certificate) bool { + return a != nil && b != nil && bytes.Equal(a.Raw, b.Raw) +} + +func (rm *resourceManager) getACMLeafCertificate( + ctx context.Context, + r *resource, +) (*x509.Certificate, error) { + if r == nil || r.ko == nil || + r.ko.Status.ACKResourceMetadata == nil || + r.ko.Status.ACKResourceMetadata.ARN == nil { + return nil, errors.New("cannot get ACM certificate without an ARN") + } + arn := string(*r.ko.Status.ACKResourceMetadata.ARN) + output, err := rm.sdkapi.GetCertificate(ctx, &svcsdk.GetCertificateInput{ + CertificateArn: &arn, + }) + rm.metrics.RecordAPICall("READ_ONE", "GetCertificate", err) + if err != nil { + return nil, err + } + if output.Certificate == nil { + return nil, errors.New("ACM GetCertificate response did not contain a certificate") + } + return parseLeafCertificate([]byte(*output.Certificate)) +} + +func setImportCertificateARN(input *svcsdk.ImportCertificateInput, r *resource) { + if input == nil || r == nil || r.ko == nil { + return + } + if input.CertificateArn != nil && *input.CertificateArn != "" { + return + } + if r.ko.Spec.CertificateARN != nil && *r.ko.Spec.CertificateARN != "" { + input.CertificateArn = r.ko.Spec.CertificateARN + return + } + if r.ko.Status.ACKResourceMetadata != nil && r.ko.Status.ACKResourceMetadata.ARN != nil { + arn := string(*r.ko.Status.ACKResourceMetadata.ARN) + input.CertificateArn = &arn + } +} + +// syncImportFromSecretIfNeeded performs at most one re-import per read. The +// next scheduled reconciliation observes ACM again, avoiding recursive sdkFind +// calls while ACM propagates the replacement certificate. +func (rm *resourceManager) syncImportFromSecretIfNeeded( + ctx context.Context, + r *resource, +) (*resource, error) { + return rm.syncImportFromSecretWithImporter( + ctx, + r, + rm.getACMLeafCertificate, + rm.ImportTlsCertificate, + ) +} + +type getACMLeafCertificateFunc func( + context.Context, + *resource, +) (*x509.Certificate, error) + +type importTLSCertificateFunc func( + context.Context, + *resource, +) (*resource, bool, error) + +func (rm *resourceManager) syncImportFromSecretWithImporter( + ctx context.Context, + r *resource, + getACMCertificate getACMLeafCertificateFunc, + importCertificate importTLSCertificateFunc, +) (*resource, error) { + if !isImportFromManagedCertificate(r) { + return r, nil + } + + leafCert, err := rm.importFromSecretLeafCertificate(ctx, r.ko.Spec) + if err != nil { + return nil, err + } + acmCert, err := getACMCertificate(ctx, r) + if err != nil { + return nil, err + } + if certificatesMatch(leafCert, acmCert) { + return r, nil + } + + rlog := ackrtlog.FromContext(ctx) + rlog.Info( + "TLS secret certificate does not match ACM certificate, re-importing", + ) + _, _, err = importCertificate(ctx, r) + if err != nil { + return nil, err + } + return nil, ackrequeue.NeededAfter( + errors.New("waiting for ACM to observe the re-imported certificate"), + 5*time.Second, + ) +} + // importCertificateInput exists as a workaround for a limitation in code-generator. // code-generator does not resolve secret key references for custom []byte fields like PrivateKey and Certificate. type importCertificateInput struct { @@ -285,6 +558,25 @@ func compareKeyAlgorithm( } } +func compareDomainName( + delta *ackcompare.Delta, + a *resource, + b *resource, +) { + // DomainName is populated by ACM for importFrom certificates and is not + // part of the user's desired state. + if a.ko.Spec.ImportFrom != nil || b.ko.Spec.ImportFrom != nil { + return + } + if ackcompare.HasNilDifference(a.ko.Spec.DomainName, b.ko.Spec.DomainName) { + delta.Add("Spec.DomainName", a.ko.Spec.DomainName, b.ko.Spec.DomainName) + } else if a.ko.Spec.DomainName != nil && b.ko.Spec.DomainName != nil { + if *a.ko.Spec.DomainName != *b.ko.Spec.DomainName { + delta.Add("Spec.DomainName", a.ko.Spec.DomainName, b.ko.Spec.DomainName) + } + } +} + func compareCertificateIssuedAt( delta *ackcompare.Delta, a *resource, diff --git a/pkg/resource/certificate/hooks_test.go b/pkg/resource/certificate/hooks_test.go new file mode 100644 index 0000000..1f10223 --- /dev/null +++ b/pkg/resource/certificate/hooks_test.go @@ -0,0 +1,305 @@ +// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"). You may +// not use this file except in compliance with the License. A copy of the +// License is located at +// +// http://aws.amazon.com/apache2.0/ +// +// or in the "license" file accompanying this file. This file is distributed +// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either +// express or implied. See the License for the specific language governing +// permissions and limitations under the License. + +package certificate + +import ( + "context" + "crypto/rand" + "crypto/rsa" + "crypto/x509" + "crypto/x509/pkix" + "encoding/pem" + "errors" + "math/big" + "testing" + "time" + + svcapitypes "github.com/aws-controllers-k8s/acm-controller/apis/v1alpha1" + ackv1alpha1 "github.com/aws-controllers-k8s/runtime/apis/core/v1alpha1" + ackrequeue "github.com/aws-controllers-k8s/runtime/pkg/requeue" + "sigs.k8s.io/controller-runtime/pkg/reconcile" +) + +type fakeReconciler struct { + secretValues map[string]string +} + +func (f *fakeReconciler) Reconcile( + context.Context, + reconcile.Request, +) (reconcile.Result, error) { + return reconcile.Result{}, nil +} + +func (f *fakeReconciler) SecretValueFromReference( + _ context.Context, + ref *ackv1alpha1.SecretKeyReference, +) (string, error) { + return f.secretValues[ref.Key], nil +} + +func (f *fakeReconciler) WriteToSecret( + context.Context, + string, + string, + string, + string, +) error { + return nil +} + +func testCertificatePEM(t *testing.T, serial int64) []byte { + t.Helper() + key, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatal(err) + } + template := &x509.Certificate{ + SerialNumber: big.NewInt(serial), + Subject: pkix.Name{CommonName: "services.k8s.aws"}, + NotBefore: time.Now().Add(-time.Minute), + NotAfter: time.Now().Add(time.Hour), + KeyUsage: x509.KeyUsageDigitalSignature, + } + der, err := x509.CreateCertificate( + rand.Reader, + template, + template, + &key.PublicKey, + key, + ) + if err != nil { + t.Fatal(err) + } + return pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: der}) +} + +func TestImportSecretRefsFromSpec(t *testing.T) { + ref := &ackv1alpha1.SecretReference{} + ref.Name = "tls-secret" + + refs, err := importSecretRefsFromSpec( + svcapitypes.CertificateSpec{ImportFrom: ref}, + ) + if err != nil { + t.Fatal(err) + } + if refs.Certificate.Key != tlsCertificateSecretDataKey { + t.Fatalf("certificate key = %q", refs.Certificate.Key) + } + if refs.PrivateKey.Key != tlsPrivateKeySecretDataKey { + t.Fatalf("private key = %q", refs.PrivateKey.Key) + } + if refs.Certificate.Name != "tls-secret" || + refs.PrivateKey.Name != "tls-secret" { + t.Fatal("secret name was not preserved") + } +} + +func TestValidateImportFromExclusivity(t *testing.T) { + importFrom := &ackv1alpha1.SecretReference{} + importFrom.Name = "tls-secret" + domainName := "example.com" + certificate := &ackv1alpha1.SecretKeyReference{Key: "certificate"} + + tests := []struct { + name string + spec svcapitypes.CertificateSpec + wantErr bool + }{ + { + name: "importFrom only", + spec: svcapitypes.CertificateSpec{ImportFrom: importFrom}, + }, + { + name: "domain name conflict", + spec: svcapitypes.CertificateSpec{ + ImportFrom: importFrom, + DomainName: &domainName, + }, + wantErr: true, + }, + { + name: "opaque certificate conflict", + spec: svcapitypes.CertificateSpec{ + ImportFrom: importFrom, + Certificate: certificate, + }, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validateImportFromExclusivity(tt.spec) + if (err != nil) != tt.wantErr { + t.Fatalf("error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} + +func TestClearImportFromObservedSpecFields(t *testing.T) { + domainName := "example.com" + keyAlgorithm := "RSA_2048" + subjectAlternativeName := "www.example.com" + certificate := &ackv1alpha1.SecretKeyReference{Key: "certificate"} + spec := svcapitypes.CertificateSpec{ + Certificate: certificate, + DomainName: &domainName, + DomainValidationOptions: []*svcapitypes.DomainValidationOption{{}}, + KeyAlgorithm: &keyAlgorithm, + Options: &svcapitypes.CertificateOptions{}, + SubjectAlternativeNames: []*string{&subjectAlternativeName}, + } + + clearImportFromObservedSpecFields(&spec) + + if spec.DomainName != nil || + spec.DomainValidationOptions != nil || + spec.KeyAlgorithm != nil || + spec.Options != nil || + spec.SubjectAlternativeNames != nil { + t.Fatal("observed request fields were not cleared") + } + if spec.Certificate != certificate { + t.Fatal("opaque import fields must not be cleared") + } +} + +func TestSplitCertificateAndChain(t *testing.T) { + leaf := testCertificatePEM(t, 1) + intermediate := testCertificatePEM(t, 2) + + certificate, chain, err := splitCertificateAndChain( + append(append([]byte{}, leaf...), intermediate...), + ) + if err != nil { + t.Fatal(err) + } + if string(certificate) != string(leaf) { + t.Fatal("leaf certificate was not preserved") + } + if string(chain) != string(intermediate) { + t.Fatal("certificate chain was not preserved") + } +} + +func TestSyncImportFromSecretIfNeededAlreadySynced(t *testing.T) { + certificatePEM := testCertificatePEM(t, 42) + acmCertificate, err := parseLeafCertificate(certificatePEM) + if err != nil { + t.Fatal(err) + } + importFrom := &ackv1alpha1.SecretReference{} + importFrom.Name = "tls-secret" + arn := ackv1alpha1.AWSResourceName("arn:aws:acm:region:account:certificate/id") + certificateType := string(svcapitypes.CertificateType_IMPORTED) + r := &resource{ko: &svcapitypes.Certificate{ + Spec: svcapitypes.CertificateSpec{ImportFrom: importFrom}, + Status: svcapitypes.CertificateStatus{ + ACKResourceMetadata: &ackv1alpha1.ResourceMetadata{ARN: &arn}, + Type: &certificateType, + }, + }} + rm := &resourceManager{rr: &fakeReconciler{ + secretValues: map[string]string{ + tlsCertificateSecretDataKey: string(certificatePEM), + }, + }} + + importCalls := 0 + got, err := rm.syncImportFromSecretWithImporter( + context.Background(), + r, + func(context.Context, *resource) (*x509.Certificate, error) { + return acmCertificate, nil + }, + func(context.Context, *resource) (*resource, bool, error) { + importCalls++ + return r, true, nil + }, + ) + if err != nil { + t.Fatal(err) + } + if got != r { + t.Fatal("already-synced resource should be returned unchanged") + } + if importCalls != 0 { + t.Fatalf("import calls = %d, want 0", importCalls) + } +} + +func TestSyncImportFromSecretIfNeededReimportsOnceAndRequeues(t *testing.T) { + certificatePEM := testCertificatePEM(t, 42) + acmCertificatePEM := testCertificatePEM(t, 42) + acmCertificate, err := parseLeafCertificate(acmCertificatePEM) + if err != nil { + t.Fatal(err) + } + secretCertificate, err := parseLeafCertificate(certificatePEM) + if err != nil { + t.Fatal(err) + } + if secretCertificate.SerialNumber.Cmp(acmCertificate.SerialNumber) != 0 { + t.Fatal("test certificates must share a serial number") + } + if certificatesMatch(secretCertificate, acmCertificate) { + t.Fatal("test certificates must have different DER encodings") + } + importFrom := &ackv1alpha1.SecretReference{} + importFrom.Name = "tls-secret" + arn := ackv1alpha1.AWSResourceName("arn:aws:acm:region:account:certificate/id") + certificateType := string(svcapitypes.CertificateType_IMPORTED) + r := &resource{ko: &svcapitypes.Certificate{ + Spec: svcapitypes.CertificateSpec{ImportFrom: importFrom}, + Status: svcapitypes.CertificateStatus{ + ACKResourceMetadata: &ackv1alpha1.ResourceMetadata{ARN: &arn}, + Type: &certificateType, + }, + }} + rm := &resourceManager{rr: &fakeReconciler{ + secretValues: map[string]string{ + tlsCertificateSecretDataKey: string(certificatePEM), + }, + }} + importCalls := 0 + + got, err := rm.syncImportFromSecretWithImporter( + context.Background(), + r, + func(context.Context, *resource) (*x509.Certificate, error) { + return acmCertificate, nil + }, + func( + context.Context, + *resource, + ) (*resource, bool, error) { + importCalls++ + return r, true, nil + }, + ) + + if got != nil { + t.Fatal("stale observed resource must not be returned after re-import") + } + if importCalls != 1 { + t.Fatalf("import calls = %d, want 1", importCalls) + } + var requeueErr *ackrequeue.RequeueNeededAfter + if !errors.As(err, &requeueErr) { + t.Fatalf("error = %T, want RequeueNeededAfter", err) + } +} diff --git a/pkg/resource/certificate/manager.go b/pkg/resource/certificate/manager.go index d54b833..bc97d7f 100644 --- a/pkg/resource/certificate/manager.go +++ b/pkg/resource/certificate/manager.go @@ -232,6 +232,9 @@ func (rm *resourceManager) LateInitialize( { observedKo := rm.concreteResource(observed).ko latestKo := rm.concreteResource(latestCopy).ko + if observedKo.Spec.DomainName != nil && latestKo.Spec.DomainName == nil { + latestKo.Spec.DomainName = observedKo.Spec.DomainName + } if observedKo.Spec.DomainValidationOptions != nil && latestKo.Spec.DomainValidationOptions == nil { latestKo.Spec.DomainValidationOptions = observedKo.Spec.DomainValidationOptions } diff --git a/pkg/resource/certificate/sdk.go b/pkg/resource/certificate/sdk.go index 68dc701..ced9e9b 100644 --- a/pkg/resource/certificate/sdk.go +++ b/pkg/resource/certificate/sdk.go @@ -366,6 +366,19 @@ func (rm *resourceManager) sdkFind( } rm.setStatusDefaults(ko) + if ko.Spec.ImportFrom != nil { + clearImportFromObservedSpecFields(&ko.Spec) + } + { + syncedRes, err := rm.syncImportFromSecretIfNeeded(ctx, &resource{ko: ko}) + if err != nil { + return nil, err + } + if syncedRes != nil && syncedRes.ko != nil { + ko = syncedRes.ko + } + } + return &resource{ko}, nil } @@ -405,7 +418,7 @@ func (rm *resourceManager) sdkCreate( defer func() { exit(err) }() - created, isImport, err := rm.maybeImportCertificate(ctx, desired) + created, isImport, err := rm.ImportTlsCertificate(ctx, desired) if err != nil { return nil, err } @@ -841,36 +854,109 @@ func (rm *resourceManager) newImportCertificateInput( input.Tags = inputf4 } + refs, err := importSecretRefsFromSpec(r.ko.Spec) + if err != nil { + return nil, err + } + { - tmpSecret, err := rm.rr.SecretValueFromReference(ctx, r.ko.Spec.PrivateKey) - if err != nil { - return nil, ackrequeue.Needed(err) - } - if tmpSecret != "" { - input.ImportCertificateInput.PrivateKey = []byte(tmpSecret) + var secretRef *ackv1alpha1.SecretKeyReference + switch "PrivateKey" { + case "PrivateKey": + secretRef = refs.PrivateKey + case "Certificate": + secretRef = refs.Certificate + case "CertificateChain": + secretRef = refs.CertificateChain + } + if secretRef != nil { + tmpSecret, err := rm.secretValueFromReference(ctx, secretRef) + if err != nil { + return nil, err + } + if tmpSecret != "" { + if "PrivateKey" == "Certificate" && refs.CertificateChain == nil { + cert, chain, err := splitCertificateAndChain([]byte(tmpSecret)) + if err != nil { + return nil, ackerr.NewTerminalError(err) + } + input.ImportCertificateInput.Certificate = cert + if len(chain) > 0 { + input.ImportCertificateInput.CertificateChain = chain + } + } else { + input.ImportCertificateInput.PrivateKey = []byte(tmpSecret) + } + } } } { - tmpSecret, err := rm.rr.SecretValueFromReference(ctx, r.ko.Spec.Certificate) - if err != nil { - return nil, ackrequeue.Needed(err) - } - if tmpSecret != "" { - input.ImportCertificateInput.Certificate = []byte(tmpSecret) + var secretRef *ackv1alpha1.SecretKeyReference + switch "Certificate" { + case "PrivateKey": + secretRef = refs.PrivateKey + case "Certificate": + secretRef = refs.Certificate + case "CertificateChain": + secretRef = refs.CertificateChain + } + if secretRef != nil { + tmpSecret, err := rm.secretValueFromReference(ctx, secretRef) + if err != nil { + return nil, err + } + if tmpSecret != "" { + if "Certificate" == "Certificate" && refs.CertificateChain == nil { + cert, chain, err := splitCertificateAndChain([]byte(tmpSecret)) + if err != nil { + return nil, ackerr.NewTerminalError(err) + } + input.ImportCertificateInput.Certificate = cert + if len(chain) > 0 { + input.ImportCertificateInput.CertificateChain = chain + } + } else { + input.ImportCertificateInput.Certificate = []byte(tmpSecret) + } + } } } { - tmpSecret, err := rm.rr.SecretValueFromReference(ctx, r.ko.Spec.CertificateChain) - if err != nil { - return nil, ackrequeue.Needed(err) - } - if tmpSecret != "" { - input.ImportCertificateInput.CertificateChain = []byte(tmpSecret) + var secretRef *ackv1alpha1.SecretKeyReference + switch "CertificateChain" { + case "PrivateKey": + secretRef = refs.PrivateKey + case "Certificate": + secretRef = refs.Certificate + case "CertificateChain": + secretRef = refs.CertificateChain + } + if secretRef != nil { + tmpSecret, err := rm.secretValueFromReference(ctx, secretRef) + if err != nil { + return nil, err + } + if tmpSecret != "" { + if "CertificateChain" == "Certificate" && refs.CertificateChain == nil { + cert, chain, err := splitCertificateAndChain([]byte(tmpSecret)) + if err != nil { + return nil, ackerr.NewTerminalError(err) + } + input.ImportCertificateInput.Certificate = cert + if len(chain) > 0 { + input.ImportCertificateInput.CertificateChain = chain + } + } else { + input.ImportCertificateInput.CertificateChain = []byte(tmpSecret) + } + } } } + setImportCertificateARN(input.ImportCertificateInput, r) + finalizeImportCertificateInput(input.ImportCertificateInput) return input.ImportCertificateInput, nil } diff --git a/templates/hooks/certificate/delta_pre_compare.go.tpl b/templates/hooks/certificate/delta_pre_compare.go.tpl index f9e6391..028f8d3 100644 --- a/templates/hooks/certificate/delta_pre_compare.go.tpl +++ b/templates/hooks/certificate/delta_pre_compare.go.tpl @@ -1,2 +1,3 @@ compareCertificateIssuedAt(delta, a, b) -compareKeyAlgorithm(delta, a, b) \ No newline at end of file +compareKeyAlgorithm(delta, a, b) +compareDomainName(delta, a, b) \ No newline at end of file diff --git a/templates/hooks/certificate/late_initialize_post_read_one.go.tpl b/templates/hooks/certificate/late_initialize_post_read_one.go.tpl index 2ea0fe9..95da6c0 100644 --- a/templates/hooks/certificate/late_initialize_post_read_one.go.tpl +++ b/templates/hooks/certificate/late_initialize_post_read_one.go.tpl @@ -1,6 +1,9 @@ { observedKo := rm.concreteResource(observed).ko latestKo := rm.concreteResource(latestCopy).ko + if observedKo.Spec.DomainName != nil && latestKo.Spec.DomainName == nil { + latestKo.Spec.DomainName = observedKo.Spec.DomainName + } if observedKo.Spec.DomainValidationOptions != nil && latestKo.Spec.DomainValidationOptions == nil { latestKo.Spec.DomainValidationOptions = observedKo.Spec.DomainValidationOptions } diff --git a/templates/hooks/certificate/sdk_create_pre_build_request.go.tpl b/templates/hooks/certificate/sdk_create_pre_build_request.go.tpl index c006e73..35ff9e2 100644 --- a/templates/hooks/certificate/sdk_create_pre_build_request.go.tpl +++ b/templates/hooks/certificate/sdk_create_pre_build_request.go.tpl @@ -1,4 +1,4 @@ - created, isImport, err := rm.maybeImportCertificate(ctx, desired) + created, isImport, err := rm.ImportTlsCertificate(ctx, desired) if err != nil { return nil, err } diff --git a/templates/hooks/certificate/sdk_file_end.go.tpl b/templates/hooks/certificate/sdk_file_end.go.tpl index c29c10a..364b73a 100644 --- a/templates/hooks/certificate/sdk_file_end.go.tpl +++ b/templates/hooks/certificate/sdk_file_end.go.tpl @@ -24,17 +24,45 @@ func (rm *resourceManager) new{{ $inputShapeName }}( ) (*svcsdk.{{ $inputShapeName }}, error) { input := &importCertificateInput{ImportCertificateInput: &svcsdk.ImportCertificateInput{}} {{ GoCodeSetSDKForStruct $CRD "" "input" $inputRef "" "r.ko.Spec" 1 }} + refs, err := importSecretRefsFromSpec(r.ko.Spec) + if err != nil { + return nil, err + } {{range $fieldName := Each "PrivateKey" "Certificate" "CertificateChain"}} { - tmpSecret, err := rm.rr.SecretValueFromReference(ctx, r.ko.Spec.{{$fieldName}}) - if err != nil { - return nil, ackrequeue.Needed(err) + var secretRef *ackv1alpha1.SecretKeyReference + switch "{{$fieldName}}" { + case "PrivateKey": + secretRef = refs.PrivateKey + case "Certificate": + secretRef = refs.Certificate + case "CertificateChain": + secretRef = refs.CertificateChain } - if tmpSecret != "" { - input.ImportCertificateInput.{{$fieldName}} = []byte(tmpSecret) + if secretRef != nil { + tmpSecret, err := rm.secretValueFromReference(ctx, secretRef) + if err != nil { + return nil, err + } + if tmpSecret != "" { + if "{{$fieldName}}" == "Certificate" && refs.CertificateChain == nil { + cert, chain, err := splitCertificateAndChain([]byte(tmpSecret)) + if err != nil { + return nil, ackerr.NewTerminalError(err) + } + input.ImportCertificateInput.Certificate = cert + if len(chain) > 0 { + input.ImportCertificateInput.CertificateChain = chain + } + } else { + input.ImportCertificateInput.{{$fieldName}} = []byte(tmpSecret) + } + } } } {{end}} + setImportCertificateARN(input.ImportCertificateInput, r) + finalizeImportCertificateInput(input.ImportCertificateInput) return input.ImportCertificateInput, nil } {{ end }} diff --git a/templates/hooks/certificate/sdk_read_one_post_set_output.go.tpl b/templates/hooks/certificate/sdk_read_one_post_set_output.go.tpl new file mode 100644 index 0000000..60d561d --- /dev/null +++ b/templates/hooks/certificate/sdk_read_one_post_set_output.go.tpl @@ -0,0 +1,12 @@ + if ko.Spec.ImportFrom != nil { + clearImportFromObservedSpecFields(&ko.Spec) + } + { + syncedRes, err := rm.syncImportFromSecretIfNeeded(ctx, &resource{ko: ko}) + if err != nil { + return nil, err + } + if syncedRes != nil && syncedRes.ko != nil { + ko = syncedRes.ko + } + } diff --git a/test/e2e/certificate.py b/test/e2e/certificate.py index 7a757a9..67c3c2b 100644 --- a/test/e2e/certificate.py +++ b/test/e2e/certificate.py @@ -118,6 +118,16 @@ def get(certificate_arn): return None +def get_body(certificate_arn): + """Returns the PEM-encoded leaf certificate from the ACM API.""" + c = boto3.client('acm') + try: + resp = c.get_certificate(CertificateArn=certificate_arn) + return resp['Certificate'] + except c.exceptions.ResourceNotFoundException: + return None + + def get_tags(certificate_arn): """Returns a dict containing the Certificate's tag records from the ACM API. diff --git a/test/e2e/resources/certificate_import_from.yaml b/test/e2e/resources/certificate_import_from.yaml new file mode 100644 index 0000000..e779804 --- /dev/null +++ b/test/e2e/resources/certificate_import_from.yaml @@ -0,0 +1,7 @@ +apiVersion: acm.services.k8s.aws/v1alpha1 +kind: Certificate +metadata: + name: $CERTIFICATE_NAME +spec: + importFrom: + name: $CERTIFICATE_NAME diff --git a/test/e2e/tests/test_certificate.py b/test/e2e/tests/test_certificate.py index 60f353c..3b9d296 100644 --- a/test/e2e/tests/test_certificate.py +++ b/test/e2e/tests/test_certificate.py @@ -41,6 +41,41 @@ MAX_WAIT_FOR_SYNCED_MINUTES = 1 +def cleanup_certificate_resource( + ref: k8s.CustomResourceReference, + fallback_arn: str = None, + secret_name: str = None, +) -> None: + cleanup_errors = [] + certificate_arn = fallback_arn + try: + if k8s.get_resource_exists(ref): + latest = k8s.get_resource(ref) + certificate_arn = latest.get('status', {}).get( + 'ackResourceMetadata', {}, + ).get('arn', certificate_arn) + _, deleted = k8s.delete_custom_resource(ref, 3, 10) + if not deleted: + raise AssertionError("certificate resource was not deleted") + except Exception as error: + cleanup_errors.append(f"custom resource cleanup failed: {error}") + + try: + if certificate_arn is not None: + certificate.wait_until_deleted(certificate_arn) + except Exception as error: + cleanup_errors.append(f"ACM certificate cleanup failed: {error}") + + if secret_name is not None: + try: + k8s.delete_secret('default', secret_name) + except Exception as error: + cleanup_errors.append(f"Secret cleanup failed: {error}") + + if cleanup_errors: + pytest.fail("; ".join(cleanup_errors)) + + @pytest.fixture def certificate_public(request) -> Tuple[k8s.CustomResourceReference, Dict]: certificate_name = random_suffix_name("certificate", 20) @@ -70,13 +105,10 @@ def certificate_public(request) -> Tuple[k8s.CustomResourceReference, Dict]: yield (ref, cr) - # Try to delete, if doesn't already exist - try: - _, deleted = k8s.delete_custom_resource(ref, 3, 10) - assert deleted - certificate.wait_until_deleted(cr["status"]["ackResourceMetadata"]["arn"]) - except: - pass + certificate_arn = cr.get('status', {}).get( + 'ackResourceMetadata', {}, + ).get('arn') + cleanup_certificate_resource(ref, certificate_arn) @pytest.fixture @@ -116,13 +148,10 @@ def certificate_import() -> Tuple[k8s.CustomResourceReference, Dict]: yield ref, cr - try: - _, deleted = k8s.delete_custom_resource(ref, 3, 10) - assert deleted - certificate.wait_until_deleted(cr['status']['ackResourceMetadata']['arn']) - k8s.delete_secret('default', certificate_name) - except: - pass + certificate_arn = cr.get('status', {}).get( + 'ackResourceMetadata', {}, + ).get('arn') + cleanup_certificate_resource(ref, certificate_arn, certificate_name) @service_marker @@ -412,6 +441,192 @@ def test_reimport_after_external_delete( time.sleep(DELETE_WAIT_AFTER_SECONDS) certificate.wait_until_deleted(new_arn) + def test_import_from_tls_secret( + self, + certificate_import_from_tls, + ): + (ref, cr) = certificate_import_from_tls + assert k8s.wait_on_condition( + ref, + condition.CONDITION_TYPE_RESOURCE_SYNCED, + "True", + wait_periods=MAX_WAIT_FOR_SYNCED_MINUTES, + ) + assert k8s.get_resource_condition(ref, condition.CONDITION_TYPE_TERMINAL) is None + + cr = k8s.get_resource(ref) + assert 'status' in cr + status = cr['status'] + assert 'ackResourceMetadata' in status + assert 'arn' in status['ackResourceMetadata'] + certificate_arn = status['ackResourceMetadata']['arn'] + assert status['type_'] == 'IMPORTED' + assert status['status'] == 'ISSUED' + assert status['subject'] == 'O=ACK,CN=services.k8s.aws' + + assert cr['spec'].get('importFrom') == {'name': ref.name} + assert certificate.get(certificate_arn) is not None + + def test_import_from_tls_reimport_after_external_delete( + self, + certificate_import_from_tls, + ): + (ref, cr) = certificate_import_from_tls + assert k8s.wait_on_condition( + ref, + condition.CONDITION_TYPE_RESOURCE_SYNCED, + "True", + wait_periods=MAX_WAIT_FOR_SYNCED_MINUTES, + ) + + cr = k8s.get_resource(ref) + original_arn = cr['status']['ackResourceMetadata']['arn'] + assert cr['status']['type_'] == 'IMPORTED' + assert cr['spec'].get('importFrom') == {'name': ref.name} + + certificate.delete(original_arn) + certificate.wait_until_deleted(original_arn) + + time.sleep(CREATE_WAIT_AFTER_SECONDS) + + assert k8s.wait_on_condition( + ref, + condition.CONDITION_TYPE_RESOURCE_SYNCED, + "True", + wait_periods=MAX_WAIT_FOR_SYNCED_MINUTES * 3, + ) + + cr = k8s.get_resource(ref) + new_arn = cr['status']['ackResourceMetadata']['arn'] + assert new_arn != original_arn + assert cr['status']['type_'] == 'IMPORTED' + assert cr['spec'].get('importFrom') == {'name': ref.name} + aws_cert = certificate.get(new_arn) + assert aws_cert is not None + assert aws_cert['Type'] == 'IMPORTED' + assert aws_cert['Status'] == 'ISSUED' + assert aws_cert['Serial'] == cr['status']['serial'] + assert certificate.get_body(new_arn) is not None + + def test_import_from_tls_reimport_after_secret_rotation( + self, + certificate_import_from_tls, + ): + (ref, _) = certificate_import_from_tls + assert k8s.wait_on_condition( + ref, + condition.CONDITION_TYPE_RESOURCE_SYNCED, + "True", + wait_periods=MAX_WAIT_FOR_SYNCED_MINUTES, + ) + + cr = k8s.get_resource(ref) + certificate_arn = cr['status']['ackResourceMetadata']['arn'] + original = certificate.get(certificate_arn) + original_serial = original['Serial'] + original_body = certificate.get_body(certificate_arn) + + replace_tls_import_secret('default', ref.name) + + observed = None + for _ in range(MAX_WAIT_FOR_SYNCED_MINUTES * 18): + observed = certificate.get(certificate_arn) + observed_body = certificate.get_body(certificate_arn) + if observed is not None and observed_body != original_body: + break + time.sleep(10) + + assert observed is not None + assert observed_body != original_body + assert observed['Serial'] != original_serial + cr = None + for _ in range(MAX_WAIT_FOR_SYNCED_MINUTES * 6): + cr = k8s.get_resource(ref) + if cr['status'].get('serial') == observed['Serial']: + break + time.sleep(10) + + assert cr is not None + assert cr['status']['serial'] == observed['Serial'] + assert k8s.wait_on_condition( + ref, + condition.CONDITION_TYPE_RESOURCE_SYNCED, + "True", + wait_periods=MAX_WAIT_FOR_SYNCED_MINUTES, + ) + assert cr['status']['ackResourceMetadata']['arn'] == certificate_arn + def k8s_client(): return k8s._get_k8s_api_client() + + +def create_tls_import_secret( + namespace: str, + name: str, + secret_type: str = 'kubernetes.io/tls', +) -> None: + private_key, cert = create_x509_certificate( + 'ACK', 'services.k8s.aws', 'acm.services.k8s.aws', + ) + body = client.V1Secret() + body.data = { + 'tls.key': base64.b64encode(private_key).decode('utf-8'), + 'tls.crt': base64.b64encode(cert).decode('utf-8'), + } + body.metadata = {'name': name} + body.type = secret_type + api_client = k8s_client() + client.CoreV1Api(api_client).create_namespaced_secret( + namespace, + api_client.sanitize_for_serialization(body), + ) + + +def replace_tls_import_secret(namespace: str, name: str) -> None: + private_key, cert = create_x509_certificate( + 'ACK', 'rotated.services.k8s.aws', 'acm.services.k8s.aws', + ) + api_client = k8s_client() + secrets = client.CoreV1Api(api_client) + secret = secrets.read_namespaced_secret(name, namespace) + secret.data = { + 'tls.key': base64.b64encode(private_key).decode('utf-8'), + 'tls.crt': base64.b64encode(cert).decode('utf-8'), + } + secret.type = 'kubernetes.io/tls' + secrets.replace_namespaced_secret(name, namespace, secret) + + +@pytest.fixture +def certificate_import_from_tls() -> Tuple[k8s.CustomResourceReference, Dict]: + certificate_name = random_suffix_name("certificate-import-from", 30) + create_tls_import_secret('default', certificate_name, 'kubernetes.io/tls') + + replacements = REPLACEMENT_VALUES.copy() + replacements['CERTIFICATE_NAME'] = certificate_name + + resource_data = load_resource( + 'certificate_import_from', + additional_replacements=replacements, + ) + + ref = k8s.CustomResourceReference( + CRD_GROUP, CRD_VERSION, RESOURCE_PLURAL, + certificate_name, namespace='default', + ) + k8s.create_custom_resource(ref, resource_data) + cr = k8s.wait_resource_consumed_by_controller(ref) + + assert cr is not None + assert k8s.get_resource_exists(ref) + assert cr['spec']['importFrom'] == {'name': certificate_name} + + time.sleep(CREATE_WAIT_AFTER_SECONDS) + + yield ref, cr + + certificate_arn = cr.get('status', {}).get( + 'ackResourceMetadata', {}, + ).get('arn') + cleanup_certificate_resource(ref, certificate_arn, certificate_name)