Skip to content

Commit

Permalink
Merge branch 'master' into master
Browse files Browse the repository at this point in the history
  • Loading branch information
Oleg Bulatov authored Aug 31, 2023
2 parents 5a7ca58 + 7a40e35 commit fe88f95
Show file tree
Hide file tree
Showing 13 changed files with 258 additions and 5 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 kustomize/components/route/quay.route.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ metadata:
quay-component: quay-app-route
annotations:
quay-component: route
haproxy.router.openshift.io/timeout: 60s
haproxy.router.openshift.io/timeout: 5m
spec:
host: $(SERVER_HOSTNAME)
to:
Expand Down
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 fe88f95

Please sign in to comment.