Skip to content

Commit

Permalink
configmaps: Track resource versions of cluster provided CA certs (PRO…
Browse files Browse the repository at this point in the history
…JQUAY-5174)

- The config.openshift.io/inject-trusted-cabundle: 'true' annotation will inject content into the config map after kustomize has generated its resources. This means that kustomize will not be aware of changes made by Openshift to these config maps
- In order to redeploy QuayRegistry resources when cluster CA are rotated, this PR implements a mechanism to track the resource version of the CA as an annotation on the Quay resources
  • Loading branch information
jonathankingfc authored and openshift-merge-robot committed Aug 30, 2023
1 parent 8aec3f2 commit 93f2e15
Show file tree
Hide file tree
Showing 12 changed files with 257 additions and 4 deletions.
2 changes: 2 additions & 0 deletions apis/quay/v1/quayregistry_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,8 @@ const (
QuayUpgradeJobName = "quay-app-upgrade"
PostgresUpgradeJobName = "quay-postgres-upgrade"
ClairPostgresUpgradeJobName = "clair-postgres-upgrade"
ClusterServiceCAName = "cluster-service-ca"
ClusterTrustedCAName = "cluster-trusted-ca"
)

// QuayRegistrySpec defines the desired state of QuayRegistry.
Expand Down
56 changes: 56 additions & 0 deletions controllers/quay/features.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ package controllers
import (
"bytes"
"context"
"crypto/sha256"
"crypto/tls"
"encoding/hex"
"encoding/pem"
"fmt"
"strings"
Expand Down Expand Up @@ -108,6 +110,60 @@ func (r *QuayRegistryReconciler) checkManagedKeys(
return nil
}

// checkClusterCAHash populates the provided QuayRegistryContext with revision version of the cluster provided CA configmaps.
// We must track these revisions so that we can force a restart of the QuayRegistry pods when the CA configmaps are updated.
func (r *QuayRegistryReconciler) checkClusterCAHash(
ctx context.Context, qctx *quaycontext.QuayRegistryContext, quay *v1.QuayRegistry,
) error {

hashConfigMapContents := func(data map[string]string, key string) string {
certData, exists := data[key]
if !exists {
return ""
}
hash := sha256.Sum256([]byte(certData))
hashStr := hex.EncodeToString(hash[:])
return hashStr[len(hashStr)-8:]
}

// Get cluster-service-ca hash
clusterServiceCAnsn := types.NamespacedName{
Name: fmt.Sprintf("%s-%s", quay.Name, v1.ClusterServiceCAName),
Namespace: quay.Namespace,
}
var clusterServiceCA corev1.ConfigMap
if err := r.Get(ctx, clusterServiceCAnsn, &clusterServiceCA); err == nil {
qctx.ClusterServiceCAHash = hashConfigMapContents(clusterServiceCA.Data, "service-ca.crt")
if currentHash, exists := clusterServiceCA.Annotations[v1.ClusterServiceCAName]; !exists || currentHash != qctx.ClusterServiceCAHash {
r.Log.Info("Detected change in cluster-service-ca configmap, updating annotation to trigger restart")
clusterServiceCA.Annotations[v1.ClusterServiceCAName] = qctx.ClusterServiceCAHash
if err := r.Update(ctx, &clusterServiceCA); err != nil {
r.Log.Error(err, "unable to update cluster-service-ca configmap annotations")
return err
}
}
}

clusterTrustedCAnsn := types.NamespacedName{
Name: fmt.Sprintf("%s-%s", quay.Name, v1.ClusterTrustedCAName),
Namespace: quay.Namespace,
}
var clusterTrustedCA corev1.ConfigMap
if err := r.Get(ctx, clusterTrustedCAnsn, &clusterTrustedCA); err == nil {
qctx.ClusterTrustedCAHash = hashConfigMapContents(clusterTrustedCA.Data, "ca-bundle.crt")
if currentHash, exists := clusterTrustedCA.Annotations[v1.ClusterTrustedCAName]; !exists || currentHash != qctx.ClusterTrustedCAHash {
r.Log.Info("Detected change in cluster-trusted-ca configmap, updating annotation to trigger restart")
clusterTrustedCA.Annotations[v1.ClusterTrustedCAName] = qctx.ClusterTrustedCAHash
if err := r.Update(ctx, &clusterTrustedCA); err != nil {
r.Log.Error(err, "unable to update cluster-trusted-ca configmap annotations")
return err
}
}
}

return nil
}

// checkManagedTLS verifies if provided bundle contains entries for ssl key and cert,
// populate the data in provided QuayRegistryContext if found.
func (r *QuayRegistryReconciler) checkManagedTLS(
Expand Down
11 changes: 11 additions & 0 deletions controllers/quay/quayregistry_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -475,6 +475,17 @@ func (r *QuayRegistryReconciler) Reconcile(ctx context.Context, req ctrl.Request
quayContext := quaycontext.NewQuayRegistryContext()
r.checkManagedTLS(quayContext, cbundle)

if err := r.checkClusterCAHash(ctx, quayContext, updatedQuay); err != nil {
return r.reconcileWithCondition(
ctx,
&quay,
v1.ConditionTypeRolloutBlocked,
metav1.ConditionTrue,
v1.ConditionReasonConfigInvalid,
fmt.Sprintf("unable to check cluster CA certs: %s", err),
)
}

if err := r.checkManagedKeys(ctx, quayContext, updatedQuay); err != nil {
return r.reconcileWithCondition(
ctx,
Expand Down
71 changes: 71 additions & 0 deletions e2e/ca_rotation/00-assert.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
apiVersion: quay.redhat.com/v1
kind: QuayRegistry
metadata:
name: ca-rotation
spec:
components:
- kind: monitoring
managed: false
- kind: mirror
managed: false
- kind: horizontalpodautoscaler
managed: false
- kind: clair
managed: false
- kind: clairpostgres
managed: false
- kind: quay
managed: true
- kind: postgres
managed: true
- kind: redis
managed: true
- kind: objectstorage
managed: true
- kind: route
managed: true
- kind: tls
managed: true
status:
conditions:
- type: ComponentHPAReady
reason: ComponentNotManaged
status: "True"
- type: ComponentRouteReady
reason: ComponentReady
status: "True"
- type: ComponentMonitoringReady
reason: ComponentNotManaged
status: "True"
- type: ComponentPostgresReady
reason: ComponentReady
status: "True"
- type: ComponentObjectStorageReady
reason: ComponentReady
status: "True"
- type: ComponentClairReady
reason: ComponentNotManaged
status: "True"
- type: ComponentClairPostgresReady
reason: ComponentNotManaged
status: "True"
- type: ComponentTLSReady
reason: ComponentReady
status: "True"
- type: ComponentRedisReady
reason: ComponentReady
status: "True"
- type: ComponentQuayReady
reason: ComponentReady
status: "True"
- type: ComponentMirrorReady
reason: ComponentNotManaged
status: "True"
- type: Available
reason: HealthChecksPassing
status: "True"
- type: ComponentsCreated
reason: ComponentsCreationSuccess
status: "True"
- type: RolloutBlocked
status: "False"
16 changes: 16 additions & 0 deletions e2e/ca_rotation/00-create-quay-registry.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
apiVersion: quay.redhat.com/v1
kind: QuayRegistry
metadata:
name: ca-rotation
spec:
components:
- kind: monitoring
managed: false
- kind: mirror
managed: false
- kind: horizontalpodautoscaler
managed: false
- kind: clair
managed: false
- kind: clairpostgres
managed: false
71 changes: 71 additions & 0 deletions e2e/ca_rotation/01-assert.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
apiVersion: quay.redhat.com/v1
kind: QuayRegistry
metadata:
name: ca-rotation
spec:
components:
- kind: monitoring
managed: false
- kind: mirror
managed: false
- kind: horizontalpodautoscaler
managed: false
- kind: clair
managed: false
- kind: clairpostgres
managed: false
- kind: quay
managed: true
- kind: postgres
managed: true
- kind: redis
managed: true
- kind: objectstorage
managed: true
- kind: route
managed: true
- kind: tls
managed: true
status:
conditions:
- type: ComponentHPAReady
reason: ComponentNotManaged
status: "True"
- type: ComponentRouteReady
reason: ComponentReady
status: "True"
- type: ComponentMonitoringReady
reason: ComponentNotManaged
status: "True"
- type: ComponentPostgresReady
reason: ComponentReady
status: "True"
- type: ComponentObjectStorageReady
reason: ComponentReady
status: "True"
- type: ComponentClairReady
reason: ComponentNotManaged
status: "True"
- type: ComponentClairPostgresReady
reason: ComponentNotManaged
status: "True"
- type: ComponentTLSReady
reason: ComponentReady
status: "True"
- type: ComponentRedisReady
reason: ComponentReady
status: "True"
- type: ComponentQuayReady
reason: ComponentReady
status: "True"
- type: ComponentMirrorReady
reason: ComponentNotManaged
status: "True"
- type: Available
reason: HealthChecksPassing
status: "True"
- type: ComponentsCreated
reason: ComponentsCreationSuccess
status: "True"
- type: RolloutBlocked
status: "False"
13 changes: 13 additions & 0 deletions e2e/ca_rotation/01-manual-rotate.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
apiVersion: kuttl.dev/v1beta1
kind: TestStep
assert:
- 01-assert.yaml
commands:
# This is how we manually rotate the cert as per https://docs.openshift.com/container-platform/4.13/security/certificates/service-serving-certificate.html#manually-rotate-service-ca_service-serving-certificate
- script: |
kubectl delete secret/signing-key -n openshift-service-ca;
for I in $(oc get ns -o jsonpath='{range .items[*]} {.metadata.name}{"\n"} {end}'); \
do oc delete pods --all -n $I; \
sleep 1; \
done
timeout: 3000
2 changes: 1 addition & 1 deletion kuttl-test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ apiVersion: kuttl.dev/v1beta1
kind: TestSuite
testDirs:
- ./e2e
timeout: 500
timeout: 600
parallel: 1
suppress:
- "events"
4 changes: 4 additions & 0 deletions pkg/context/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ type QuayRegistryContext struct {
ServerHostname string
BuildManagerHostname string

// Cluster CA Resource Versions
ClusterServiceCAHash string
ClusterTrustedCAHash string

// TLS
ClusterWildcardCert []byte
TLSCert []byte
Expand Down
2 changes: 1 addition & 1 deletion pkg/kustomize/kustomize.go
Original file line number Diff line number Diff line change
Expand Up @@ -651,7 +651,7 @@ func Inflate(
}

for index, resource := range resources {
obj, err := middleware.Process(quay, resource, skipres)
obj, err := middleware.Process(quay, ctx, resource, skipres)
if err != nil {
return nil, err
}
Expand Down
9 changes: 8 additions & 1 deletion pkg/middleware/middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"strings"

route "github.com/openshift/api/route/v1"
quaycontext "github.com/quay/quay-operator/pkg/context"
appsv1 "k8s.io/api/apps/v1"
batchv1 "k8s.io/api/batch/v1"
corev1 "k8s.io/api/core/v1"
Expand All @@ -27,7 +28,7 @@ const (
// Process applies any additional middleware steps to a managed k8s object that cannot be
// accomplished using the Kustomize toolchain. if skipres is set all resource requests are
// trimmed from the objects thus deploying quay with a much smaller footprint.
func Process(quay *v1.QuayRegistry, obj client.Object, skipres bool) (client.Object, error) {
func Process(quay *v1.QuayRegistry, qctx *quaycontext.QuayRegistryContext, obj client.Object, skipres bool) (client.Object, error) {
objectMeta, err := meta.Accessor(obj)
if err != nil {
return nil, err
Expand Down Expand Up @@ -89,6 +90,12 @@ func Process(quay *v1.QuayRegistry, obj client.Object, skipres bool) (client.Obj
dep.Spec.Template.Spec.Affinity = oaff
}

// Add annotations to track the hash of the cluster service CA. This is to ensure that we redeploy when the cluster service CA changes.
dep.Annotations[v1.ClusterServiceCAName] = qctx.ClusterServiceCAHash
dep.Annotations[v1.ClusterTrustedCAName] = qctx.ClusterTrustedCAHash
dep.Spec.Template.Annotations[v1.ClusterServiceCAName] = qctx.ClusterServiceCAHash
dep.Spec.Template.Annotations[v1.ClusterTrustedCAName] = qctx.ClusterTrustedCAHash

// here we do an attempt to setting the default or overwriten number of replicas
// for clair, quay and mirror. we can't do that if horizontal pod autoscaler is
// in managed state as we would be stomping in the values defined by the hpa
Expand Down
4 changes: 3 additions & 1 deletion pkg/middleware/middleware_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"testing"

route "github.com/openshift/api/route/v1"
quaycontext "github.com/quay/quay-operator/pkg/context"
"github.com/stretchr/testify/assert"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
Expand Down Expand Up @@ -230,7 +231,8 @@ func TestProcess(t *testing.T) {
for _, test := range processTests {

t.Run(test.name, func(t *testing.T) {
processedObj, err := Process(test.quay, test.obj, false)
quayContext := quaycontext.NewQuayRegistryContext()
processedObj, err := Process(test.quay, quayContext, test.obj, false)
if test.expectedError != nil {
assert.Error(err, test.name)
} else {
Expand Down

0 comments on commit 93f2e15

Please sign in to comment.