Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

K8SPG-555: fix updating CA secrets to 2.5.0 #906

Merged
merged 9 commits into from
Sep 27, 2024
Merged
Show file tree
Hide file tree
Changes from 5 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
32 changes: 26 additions & 6 deletions internal/controller/postgrescluster/pki.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
"github.com/pkg/errors"
appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
k8serrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"sigs.k8s.io/controller-runtime/pkg/client"

"github.com/percona/percona-postgresql-operator/internal/naming"
Expand Down Expand Up @@ -69,8 +71,22 @@
}
}

err := errors.WithStack(client.IgnoreNotFound(
r.Client.Get(ctx, client.ObjectKeyFromObject(existing), existing)))
err := errors.WithStack(
r.Client.Get(ctx, client.ObjectKeyFromObject(existing), existing))
// K8SPG-555: we need to check ca certificate from old operator versions
// TODO: remove when 2.4.0 will become unsupported
if k8serrors.IsNotFound(err) {
nn := client.ObjectKeyFromObject(existing)
nn.Name = naming.RootCertSecret

Check failure on line 80 in internal/controller/postgrescluster/pki.go

View workflow job for this annotation

GitHub Actions / runner / suggester / golangci-lint

SA1019: naming.RootCertSecret is deprecated: K8SPG-555: use PostgresRootCASecret instead. Currently it's used to update certificates from older operator version RootCertSecret is the default root certificate secret name TODO: remove when 2.4.0 will become unsupported (staticcheck)
err = errors.WithStack(
r.Client.Get(ctx, nn, existing))
if err == nil {
existing.Name = naming.RootCertSecret

Check failure on line 84 in internal/controller/postgrescluster/pki.go

View workflow job for this annotation

GitHub Actions / runner / suggester / golangci-lint

SA1019: naming.RootCertSecret is deprecated: K8SPG-555: use PostgresRootCASecret instead. Currently it's used to update certificates from older operator version RootCertSecret is the default root certificate secret name TODO: remove when 2.4.0 will become unsupported (staticcheck)
}
}
if k8serrors.IsNotFound(err) {
err = nil
}

root := &pki.RootCertificateAuthority{}

Expand All @@ -81,17 +97,21 @@
_ = root.Certificate.UnmarshalText(existing.Data[certificateKey])
_ = root.PrivateKey.UnmarshalText(existing.Data[privateKey])

if cluster.Spec.CustomRootCATLSSecret != nil {
return root, err
}

if !pki.RootIsValid(root) {
root, err = pki.NewRootCertificateAuthority()
err = errors.WithStack(err)
}
}
if cluster.Spec.CustomRootCATLSSecret != nil {
return root, err
}

intent := &corev1.Secret{
ObjectMeta: naming.PostgresRootCASecret(cluster),
ObjectMeta: metav1.ObjectMeta{
Name: existing.Name,
Namespace: existing.Namespace,
},
}
intent.SetGroupVersionKind(corev1.SchemeGroupVersion.WithKind("Secret"))
intent.Data = make(map[string][]byte)
Expand Down
6 changes: 4 additions & 2 deletions internal/naming/names.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,11 @@ const (
)

const (
// K8SPG-555: use PostgresRootCASecret instead.
// Deprecated: K8SPG-555: use PostgresRootCASecret instead.
// Currently it's used to update certificates from older operator version
// RootCertSecret is the default root certificate secret name
// RootCertSecret = "pgo-root-cacert" /* #nosec */
// TODO: remove when 2.4.0 will become unsupported
RootCertSecret = "pgo-root-cacert" /* #nosec */

// ClusterCertSecret is the default cluster leaf certificate secret name
ClusterCertSecret = "%s-cluster-cert" /* #nosec */
Expand Down
97 changes: 97 additions & 0 deletions percona/controller/pgcluster/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,10 @@
return reconcile.Result{}, nil
}

if err := r.reconcileTLS(ctx, cr); err != nil {
return reconcile.Result{}, errors.Wrap(err, "reconcile TLS")
}

if err := r.reconcileExternalWatchers(ctx, cr); err != nil {
return reconcile.Result{}, errors.Wrap(err, "start external watchers")
}
Expand Down Expand Up @@ -282,6 +286,99 @@
return ctrl.Result{}, nil
}

func (r *PGClusterReconciler) reconcileTLS(ctx context.Context, cr *v2.PerconaPGCluster) error {
checkSecretProjection := func(p *corev1.SecretProjection, requiredPaths ...string) error {
if p == nil {
return nil
}

if p.Name == "" {
return errors.New("secret name is not specified")
}

secret := new(corev1.Secret)
nn := types.NamespacedName{Name: p.Name, Namespace: cr.Namespace}
if err := r.Client.Get(ctx, nn, secret); err != nil {
return errors.Wrapf(err, "failed to get secret %s", nn.Name)
}

pathMap := make(map[string]struct{})
for _, item := range p.Items {
if _, ok := secret.Data[item.Key]; !ok {
return errors.Errorf("key %s doesn't exist in secret %s", item.Key, secret.Name)
}
pathMap[item.Path] = struct{}{}
}

for _, path := range requiredPaths {
if _, ok := pathMap[path]; !ok {
return errors.Errorf("required path %s was not found", path)
}
}

return nil
}

if err := checkSecretProjection(cr.Spec.Secrets.CustomRootCATLSSecret, "root.crt", "root.key"); err != nil {
return errors.Wrap(err, "failed to validate .spec.customRootCATLSSecret")
}

certPaths := []string{"tls.key", "tls.crt"}
if cr.Spec.Secrets.CustomRootCATLSSecret == nil {
certPaths = append(certPaths, "ca.crt")
}
if err := checkSecretProjection(cr.Spec.Secrets.CustomTLSSecret, certPaths...); err != nil {
return errors.Wrap(err, "failed to validate .spec.customTLSSecret")
}
if err := checkSecretProjection(cr.Spec.Secrets.CustomReplicationClientTLSSecret, certPaths...); err != nil {
return errors.Wrap(err, "failed to validate .spec.customReplicationTLSSecret")
}

oldCASecret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: naming.RootCertSecret,

Check failure on line 339 in percona/controller/pgcluster/controller.go

View workflow job for this annotation

GitHub Actions / runner / suggester / golangci-lint

SA1019: naming.RootCertSecret is deprecated: K8SPG-555: use PostgresRootCASecret instead. Currently it's used to update certificates from older operator version RootCertSecret is the default root certificate secret name TODO: remove when 2.4.0 will become unsupported (staticcheck)
Namespace: cr.Namespace,
},
}
err := r.Client.Get(ctx, client.ObjectKeyFromObject(oldCASecret), oldCASecret)
if client.IgnoreNotFound(err) != nil {
return errors.Wrap(err, "failed to get old ca secret")
}

if cr.CompareVersion("2.5.0") < 0 {
if k8serrors.IsNotFound(err) {
// K8SPG-555: We should create an empty secret with old name, so that crunchy part can populate it
// instead of creating secrets unique to the cluster
// TODO: remove when 2.4.0 will become unsupported
if err := r.Client.Create(ctx, oldCASecret); err != nil {
return errors.Wrap(err, "failed to create ca secret")
}
}
return nil
}
if k8serrors.IsNotFound(err) {
return nil
}
Comment on lines +362 to +375
Copy link
Contributor

Choose a reason for hiding this comment

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

can we merge these two conditions:

Suggested change
if cr.CompareVersion("2.5.0") < 0 {
if k8serrors.IsNotFound(err) {
// K8SPG-555: We should create an empty secret with old name, so that crunchy part can populate it
// instead of creating secrets unique to the cluster
// TODO: remove when 2.4.0 will become unsupported
if err := r.Client.Create(ctx, oldCASecret); err != nil {
return errors.Wrap(err, "failed to create ca secret")
}
}
return nil
}
if k8serrors.IsNotFound(err) {
return nil
}
if k8serrors.IsNotFound(err) {
if cr.CompareVersion("2.5.0") < 0 {
// K8SPG-555: We should create an empty secret with old name, so that crunchy part can populate it
// instead of creating secrets unique to the cluster
// TODO: remove when 2.4.0 will become unsupported
if err := r.Client.Create(ctx, oldCASecret); err != nil {
return errors.Wrap(err, "failed to create ca secret")
}
}
return nil
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can't merge it, because we have to return if the version is lower than 2.5.0, even if secret was found.


// K8SPG-555: Previously we used a single CA secret for all clusters in a namespace.
// We should copy the contents of the old CA secret, if it exists, to the new one, which is unique for each cluster.
// TODO: remove when 2.4.0 will become unsupported
newCASecret := &corev1.Secret{
ObjectMeta: naming.PostgresRootCASecret(&v1beta1.PostgresCluster{ObjectMeta: metav1.ObjectMeta{Name: cr.Name, Namespace: cr.Namespace}}),
Copy link
Contributor

Choose a reason for hiding this comment

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

can we break this into multiple lines?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

}
err = r.Client.Get(ctx, client.ObjectKeyFromObject(newCASecret), new(corev1.Secret))
if client.IgnoreNotFound(err) != nil {
return errors.Wrap(err, "failed to get new ca secret")
}
if k8serrors.IsNotFound(err) {
newCASecret.Data = oldCASecret.Data
if err := r.Client.Create(ctx, newCASecret); err != nil {
return errors.Wrap(err, "failed to create updated CA secret")
}
}
return nil
}

func (r *PGClusterReconciler) reconcilePMM(ctx context.Context, cr *v2.PerconaPGCluster) error {
if !cr.PMMEnabled() {
return nil
Expand Down
1 change: 1 addition & 0 deletions percona/controller/pgcluster/finalizer.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ func (r *PGClusterReconciler) deleteTLSSecrets(ctx context.Context, cr *v2.Perco
naming.ClusterPGBouncer(crunchyCluster),
}
if cr.Spec.Secrets.CustomRootCATLSSecret == nil {
secretsMeta = append(secretsMeta, metav1.ObjectMeta{Namespace: crunchyCluster.Namespace, Name: naming.RootCertSecret})
secretsMeta = append(secretsMeta, naming.PostgresRootCASecret(crunchyCluster))
}
if cr.Spec.Secrets.CustomTLSSecret == nil {
Expand Down
Loading