Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion api/v1alpha1/etcdcluster_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,8 @@ type CommonConfig struct {
AltNames AltNames `json:"altNames,omitempty"`

// ValidityDuration is the expected duration until which the certificate will be valid,
// expects in human-readable duration: 100d12h, if empty defaults to 90d
// expects in human-readable duration: 100d12h, if empty defaults to 90d for cert-manager
// and 365d for auto as per: https://github.com/etcd-io/etcd/blob/b87bc1c3a275d7d4904f4d201b963a2de2264f0d/client/pkg/transport/listener.go#L275
// +optional
ValidityDuration string `json:"validityDuration,omitempty"`

Expand Down
6 changes: 4 additions & 2 deletions config/crd/bases/operator.etcd.io_etcdclusters.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,8 @@ spec:
validityDuration:
description: |-
ValidityDuration is the expected duration until which the certificate will be valid,
expects in human-readable duration: 100d12h, if empty defaults to 90d
expects in human-readable duration: 100d12h, if empty defaults to 90d for cert-manager
and 365d for auto as per: https://github.com/etcd-io/etcd/blob/b87bc1c3a275d7d4904f4d201b963a2de2264f0d/client/pkg/transport/listener.go#L275
type: string
type: object
certManagerCfg:
Expand Down Expand Up @@ -203,7 +204,8 @@ spec:
validityDuration:
description: |-
ValidityDuration is the expected duration until which the certificate will be valid,
expects in human-readable duration: 100d12h, if empty defaults to 90d
expects in human-readable duration: 100d12h, if empty defaults to 90d for cert-manager
and 365d for auto as per: https://github.com/etcd-io/etcd/blob/b87bc1c3a275d7d4904f4d201b963a2de2264f0d/client/pkg/transport/listener.go#L275
type: string
required:
- issuerKind
Expand Down
109 changes: 82 additions & 27 deletions internal/controller/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func reconcileStatefulSet(ctx context.Context, logger logr.Logger, ec *ecv1alpha
}

// Add server and peer certificate
err = applyEtcdMemberCerts(ctx, ec, c, logger)
err = applyEtcdMemberCerts(ctx, ec, c)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -600,13 +600,42 @@ func createCMCertificateConfig(ec *ecv1alpha1.EtcdCluster) *certInterface.Config
}

func createAutoCertificateConfig(ec *ecv1alpha1.EtcdCluster) *certInterface.Config {
// TODO
config := &certInterface.Config{}
autoConfig := ec.Spec.TLS.ProviderCfg.AutoCfg
duration, err := time.ParseDuration(autoConfig.ValidityDuration)
if err != nil {
log.Printf("Failed to parse ValidityDuration: %s", err)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should it return an error here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am currently handling the scenario in auto/provider.go - https://github.com/ArkaSaha30/etcd-operator/blob/auto-cert-provider/pkg/certificate/auto/provider.go#L217-L220
Currently, it will log the error but fall back to the default duration of 1yr and let the user bring up the etcdCluster.
Please let me know if this is not the desired behaviour, and I will update it to throw an error.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should return an error here, we only use the default value if not provided. Pls raise a separate ticket to track this.

}

var altNames certInterface.AltNames
if autoConfig.AltNames.DNSNames != nil {
altNames = certInterface.AltNames{
DNSNames: autoConfig.AltNames.DNSNames,
IPs: make([]net.IP, len(autoConfig.AltNames.DNSNames)),
}
} else {
defaultDNSNames := []string{fmt.Sprintf("%s.svc.cluster.local", autoConfig.CommonName)}
altNames = certInterface.AltNames{
DNSNames: defaultDNSNames,
}
}

config := &certInterface.Config{
CommonName: autoConfig.CommonName,
Organization: autoConfig.Organization,
ValidityDuration: duration,
AltNames: altNames,
}
return config
}

func createCertificate(ec *ecv1alpha1.EtcdCluster, ctx context.Context, c client.Client, certName string) error {
cert, certErr := certificate.NewProvider(certificate.ProviderType(ec.Spec.TLS.Provider), c)
// The TLS field is present but spec is empty
providerName := ec.Spec.TLS.Provider
if providerName == "" {
providerName = string(certificate.Auto)
}

cert, certErr := certificate.NewProvider(certificate.ProviderType(providerName), c)
if certErr != nil {
// TODO: instead of error, set default autoConfig
return certErr
Expand All @@ -617,8 +646,8 @@ func createCertificate(ec *ecv1alpha1.EtcdCluster, ctx context.Context, c client
log.Printf("Creating certificate: %s for etcd-operator: %s\n", certName, ec.Name)
switch {
case ec.Spec.TLS.ProviderCfg.AutoCfg != nil:
cmConfig := createAutoCertificateConfig(ec)
createCertErr := cert.EnsureCertificateSecret(ctx, certName, ec.Namespace, cmConfig)
autoConfig := createAutoCertificateConfig(ec)
createCertErr := cert.EnsureCertificateSecret(ctx, certName, ec.Namespace, autoConfig)
if createCertErr != nil {
log.Printf("Error creating certificate: %s", createCertErr)
}
Expand All @@ -645,44 +674,70 @@ func createCertificate(ec *ecv1alpha1.EtcdCluster, ctx context.Context, c client

func createClientCertificate(ctx context.Context, ec *ecv1alpha1.EtcdCluster, c client.Client) error {
certName := getClientCertName(ec.Name)
createClientCertErr := createCertificate(ec, ctx, c, certName)
return createClientCertErr
err := createCertificate(ec, ctx, c, certName)
if err != nil {
return err
}
err = patchCertificateSecret(ctx, ec, c, certName)
if err != nil {
return fmt.Errorf("patching certificate secret: %s with ownerReference failed: %w", certName, err)
}
return err
}

func createServerCertificate(ctx context.Context, ec *ecv1alpha1.EtcdCluster, c client.Client) error {
serverCertName := getServerCertName(ec.Name)
createServerCertErr := createCertificate(ec, ctx, c, serverCertName)
if createServerCertErr != nil {
return createServerCertErr
err := createCertificate(ec, ctx, c, serverCertName)
if err != nil {
return err
}
err = patchCertificateSecret(ctx, ec, c, serverCertName)
if err != nil {
return fmt.Errorf("patching certificate secret: %s with ownerReference failed: %w", serverCertName, err)
}
return nil
}

func createPeerCertificate(ctx context.Context, ec *ecv1alpha1.EtcdCluster, c client.Client) error {
peerCertName := getPeerCertName(ec.Name)
createPeerCertErr := createCertificate(ec, ctx, c, peerCertName)
if createPeerCertErr != nil {
return createPeerCertErr
err := createCertificate(ec, ctx, c, peerCertName)
if err != nil {
return err
}
err = patchCertificateSecret(ctx, ec, c, peerCertName)
if err != nil {
return fmt.Errorf("patching certificate secret: %s with ownerReference failed: %w", peerCertName, err)
}
return nil
}

func applyEtcdMemberCerts(ctx context.Context, ec *ecv1alpha1.EtcdCluster, c client.Client, logger logr.Logger) error {
var err error
func applyEtcdMemberCerts(ctx context.Context, ec *ecv1alpha1.EtcdCluster, c client.Client) error {
if ec.Spec.TLS != nil {
createServerCertErr := createServerCertificate(ctx, ec, c)
if createServerCertErr != nil {
err = createServerCertErr
logger.Error(createServerCertErr, "Error creating Server Certificate")

err := createServerCertificate(ctx, ec, c)
if err != nil {
return err
}
createPeerCertErr := createPeerCertificate(ctx, ec, c)
if createPeerCertErr != nil {
err = createPeerCertErr
logger.Error(createPeerCertErr, "Error creating Peer Certificate")

err = createPeerCertificate(ctx, ec, c)
if err != nil {
return err
}
}
return nil
}

func patchCertificateSecret(ctx context.Context, ec *ecv1alpha1.EtcdCluster, c client.Client, certSecretName string) error {
getCertSecret := &corev1.Secret{}
if err := c.Get(ctx, client.ObjectKey{Name: certSecretName, Namespace: ec.Namespace}, getCertSecret); err != nil {
return err
}
return err

log.Printf("Setting ownerReference for certificate secret: %s", certSecretName)
if err := controllerutil.SetControllerReference(ec, getCertSecret, c.Scheme()); err != nil {
return err
}
if err := c.Update(ctx, getCertSecret); err != nil {
return fmt.Errorf("failed to update certificate secret with ownerReference: %w", err)
}

return nil
}
Loading