Skip to content

Commit 7811462

Browse files
ecordellecordell
committed
Add a label to secrets managed by OLM, and don't watch secrets that omit
the label. This should greatly reduce the secret cache size. Signed-off-by: Evan <[email protected]>
1 parent 2d11302 commit 7811462

File tree

5 files changed

+260
-8
lines changed

5 files changed

+260
-8
lines changed

‎pkg/controller/install/certresources.go

+15-4
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,9 @@ const (
3939
Organization = "Red Hat, Inc."
4040
// Kubernetes System namespace
4141
KubeSystem = "kube-system"
42+
// olm managed label
43+
OLMManagedLabelKey = "olm.managed"
44+
OLMManagedLabelValue = "true"
4245
)
4346

4447
type certResource interface {
@@ -292,6 +295,7 @@ func (i *StrategyDeploymentInstaller) installCertRequirementsForDeployment(deplo
292295
secret.SetName(SecretName(service.GetName()))
293296
secret.SetNamespace(i.owner.GetNamespace())
294297
secret.SetAnnotations(map[string]string{OLMCAHashAnnotationKey: caHash})
298+
secret.SetLabels(map[string]string{OLMManagedLabelKey: OLMManagedLabelValue})
295299

296300
existingSecret, err := i.strategyClient.GetOpLister().CoreV1().SecretLister().Secrets(i.owner.GetNamespace()).Get(secret.GetName())
297301
if err == nil {
@@ -315,10 +319,17 @@ func (i *StrategyDeploymentInstaller) installCertRequirementsForDeployment(deplo
315319
} else if k8serrors.IsNotFound(err) {
316320
// Create the secret
317321
ownerutil.AddNonBlockingOwner(secret, i.owner)
318-
_, err = i.strategyClient.GetOpClient().CreateSecret(secret)
319-
if err != nil {
320-
log.Warnf("could not create secret %s", secret.GetName())
321-
return nil, nil, err
322+
if _, err := i.strategyClient.GetOpClient().CreateSecret(secret); err != nil {
323+
if !k8serrors.IsAlreadyExists(err) {
324+
log.Warnf("could not create secret %s: %v", secret.GetName(), err)
325+
return nil, nil, err
326+
}
327+
// if the secret isn't in the cache but exists in the cluster, it's missing the labels for the cache filter
328+
// and just needs to be updated
329+
if _, err := i.strategyClient.GetOpClient().UpdateSecret(secret); err != nil {
330+
log.Warnf("could not update secret %s: %v", secret.GetName(), err)
331+
return nil, nil, err
332+
}
322333
}
323334
} else {
324335
return nil, nil, err

‎pkg/controller/install/certresources_test.go

+239
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,9 @@ import (
1212
appsv1 "k8s.io/api/apps/v1"
1313
corev1 "k8s.io/api/core/v1"
1414
rbacv1 "k8s.io/api/rbac/v1"
15+
"k8s.io/apimachinery/pkg/api/errors"
1516
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
17+
"k8s.io/apimachinery/pkg/runtime/schema"
1618

1719
"github.com/operator-framework/api/pkg/operators/v1alpha1"
1820
"github.com/operator-framework/operator-lifecycle-manager/pkg/api/wrappers"
@@ -182,6 +184,7 @@ func TestInstallCertRequirementsForDeployment(t *testing.T) {
182184
Name: "test-service-cert",
183185
Namespace: namespace,
184186
Annotations: map[string]string{OLMCAHashAnnotationKey: caHash},
187+
Labels: map[string]string{OLMManagedLabelKey: OLMManagedLabelValue},
185188
},
186189
Data: map[string][]byte{
187190
"tls.crt": certPEM,
@@ -405,6 +408,7 @@ func TestInstallCertRequirementsForDeployment(t *testing.T) {
405408
Name: "test-service-cert",
406409
Namespace: namespace,
407410
Annotations: map[string]string{OLMCAHashAnnotationKey: caHash},
411+
Labels: map[string]string{OLMManagedLabelKey: OLMManagedLabelValue},
408412
},
409413
Data: map[string][]byte{
410414
"tls.crt": certPEM,
@@ -587,6 +591,241 @@ func TestInstallCertRequirementsForDeployment(t *testing.T) {
587591
},
588592
},
589593
},
594+
{
595+
name: "labels an unlabelled secret if present",
596+
mockExternal: func(mockOpClient *operatorclientmocks.MockClientInterface, fakeLister *operatorlisterfakes.FakeOperatorLister, namespace string, args args) {
597+
mockOpClient.EXPECT().DeleteService(namespace, "test-service", &metav1.DeleteOptions{}).Return(nil)
598+
service := corev1.Service{
599+
ObjectMeta: metav1.ObjectMeta{
600+
Name: "test-service",
601+
OwnerReferences: []metav1.OwnerReference{
602+
ownerutil.NonBlockingOwner(&v1alpha1.ClusterServiceVersion{}),
603+
},
604+
},
605+
Spec: corev1.ServiceSpec{
606+
Ports: args.ports,
607+
Selector: selector(t, "test=label").MatchLabels,
608+
},
609+
}
610+
mockOpClient.EXPECT().CreateService(&service).Return(&service, nil)
611+
612+
hosts := []string{
613+
fmt.Sprintf("%s.%s", service.GetName(), namespace),
614+
fmt.Sprintf("%s.%s.svc", service.GetName(), namespace),
615+
}
616+
servingPair, err := certGenerator.Generate(args.rotateAt, Organization, args.ca, hosts)
617+
require.NoError(t, err)
618+
619+
// Create Secret for serving cert
620+
certPEM, privPEM, err := servingPair.ToPEM()
621+
require.NoError(t, err)
622+
623+
secret := &corev1.Secret{
624+
ObjectMeta: metav1.ObjectMeta{
625+
Name: "test-service-cert",
626+
Namespace: namespace,
627+
Annotations: map[string]string{OLMCAHashAnnotationKey: caHash},
628+
Labels: map[string]string{OLMManagedLabelKey: OLMManagedLabelValue},
629+
OwnerReferences: []metav1.OwnerReference{
630+
ownerutil.NonBlockingOwner(&v1alpha1.ClusterServiceVersion{}),
631+
},
632+
},
633+
Data: map[string][]byte{
634+
"tls.crt": certPEM,
635+
"tls.key": privPEM,
636+
OLMCAPEMKey: caPEM,
637+
},
638+
Type: corev1.SecretTypeTLS,
639+
}
640+
// secret already exists, but without label
641+
mockOpClient.EXPECT().CreateSecret(secret).Return(nil, errors.NewAlreadyExists(schema.GroupResource{
642+
Group: "",
643+
Resource: "secrets",
644+
}, "test-service-cert"))
645+
646+
// update secret with label
647+
mockOpClient.EXPECT().UpdateSecret(secret).Return(secret, nil)
648+
649+
secretRole := &rbacv1.Role{
650+
ObjectMeta: metav1.ObjectMeta{
651+
Name: secret.GetName(),
652+
Namespace: namespace,
653+
},
654+
Rules: []rbacv1.PolicyRule{
655+
{
656+
Verbs: []string{"get"},
657+
APIGroups: []string{""},
658+
Resources: []string{"secrets"},
659+
ResourceNames: []string{secret.GetName()},
660+
},
661+
},
662+
}
663+
mockOpClient.EXPECT().UpdateRole(secretRole).Return(secretRole, nil)
664+
665+
roleBinding := &rbacv1.RoleBinding{
666+
ObjectMeta: metav1.ObjectMeta{
667+
Name: secret.GetName(),
668+
Namespace: namespace,
669+
},
670+
Subjects: []rbacv1.Subject{
671+
{
672+
Kind: "ServiceAccount",
673+
APIGroup: "",
674+
Name: "test-sa",
675+
Namespace: namespace,
676+
},
677+
},
678+
RoleRef: rbacv1.RoleRef{
679+
APIGroup: "rbac.authorization.k8s.io",
680+
Kind: "Role",
681+
Name: secretRole.GetName(),
682+
},
683+
}
684+
mockOpClient.EXPECT().UpdateRoleBinding(roleBinding).Return(roleBinding, nil)
685+
686+
authDelegatorClusterRoleBinding := &rbacv1.ClusterRoleBinding{
687+
ObjectMeta: metav1.ObjectMeta{
688+
Name: service.GetName() + "-system:auth-delegator",
689+
},
690+
Subjects: []rbacv1.Subject{
691+
{
692+
Kind: "ServiceAccount",
693+
APIGroup: "",
694+
Name: "test-sa",
695+
Namespace: namespace,
696+
},
697+
},
698+
RoleRef: rbacv1.RoleRef{
699+
APIGroup: "rbac.authorization.k8s.io",
700+
Kind: "ClusterRole",
701+
Name: "system:auth-delegator",
702+
},
703+
}
704+
705+
mockOpClient.EXPECT().UpdateClusterRoleBinding(authDelegatorClusterRoleBinding).Return(authDelegatorClusterRoleBinding, nil)
706+
707+
authReaderRoleBinding := &rbacv1.RoleBinding{
708+
Subjects: []rbacv1.Subject{
709+
{
710+
Kind: "ServiceAccount",
711+
APIGroup: "",
712+
Name: args.depSpec.Template.Spec.ServiceAccountName,
713+
Namespace: namespace,
714+
},
715+
},
716+
RoleRef: rbacv1.RoleRef{
717+
APIGroup: "rbac.authorization.k8s.io",
718+
Kind: "Role",
719+
Name: "extension-apiserver-authentication-reader",
720+
},
721+
}
722+
authReaderRoleBinding.SetName(service.GetName() + "-auth-reader")
723+
authReaderRoleBinding.SetNamespace(KubeSystem)
724+
725+
mockOpClient.EXPECT().UpdateRoleBinding(authReaderRoleBinding).Return(authReaderRoleBinding, nil)
726+
},
727+
state: fakeState{
728+
existingService: &corev1.Service{
729+
ObjectMeta: metav1.ObjectMeta{
730+
OwnerReferences: []metav1.OwnerReference{
731+
ownerutil.NonBlockingOwner(&v1alpha1.ClusterServiceVersion{}),
732+
},
733+
},
734+
},
735+
// unlabelled secret won't be in cache
736+
getSecretError: errors.NewNotFound(schema.GroupResource{
737+
Group: "",
738+
Resource: "Secret",
739+
}, "nope"),
740+
existingRole: &rbacv1.Role{
741+
ObjectMeta: metav1.ObjectMeta{},
742+
},
743+
existingRoleBinding: &rbacv1.RoleBinding{
744+
ObjectMeta: metav1.ObjectMeta{},
745+
},
746+
existingClusterRoleBinding: &rbacv1.ClusterRoleBinding{
747+
ObjectMeta: metav1.ObjectMeta{},
748+
},
749+
},
750+
fields: fields{
751+
owner: &v1alpha1.ClusterServiceVersion{},
752+
previousStrategy: nil,
753+
templateAnnotations: nil,
754+
initializers: nil,
755+
apiServiceDescriptions: []certResource{},
756+
webhookDescriptions: []certResource{},
757+
},
758+
args: args{
759+
deploymentName: "test",
760+
ca: ca,
761+
rotateAt: time.Now().Add(time.Hour),
762+
ports: []corev1.ServicePort{},
763+
depSpec: appsv1.DeploymentSpec{
764+
Selector: selector(t, "test=label"),
765+
Template: corev1.PodTemplateSpec{
766+
Spec: corev1.PodSpec{
767+
ServiceAccountName: "test-sa",
768+
},
769+
ObjectMeta: metav1.ObjectMeta{
770+
Annotations: map[string]string{
771+
"foo": "bar",
772+
},
773+
},
774+
},
775+
},
776+
},
777+
want: &appsv1.DeploymentSpec{
778+
Selector: selector(t, "test=label"),
779+
Template: corev1.PodTemplateSpec{
780+
ObjectMeta: metav1.ObjectMeta{
781+
Annotations: map[string]string{
782+
"foo": "bar",
783+
OLMCAHashAnnotationKey: caHash},
784+
},
785+
Spec: corev1.PodSpec{
786+
ServiceAccountName: "test-sa",
787+
Volumes: []corev1.Volume{
788+
{
789+
Name: "apiservice-cert",
790+
VolumeSource: corev1.VolumeSource{
791+
Secret: &corev1.SecretVolumeSource{
792+
SecretName: "test-service-cert",
793+
Items: []corev1.KeyToPath{
794+
{
795+
Key: "tls.crt",
796+
Path: "apiserver.crt",
797+
},
798+
{
799+
Key: "tls.key",
800+
Path: "apiserver.key",
801+
},
802+
},
803+
},
804+
},
805+
},
806+
{
807+
Name: "webhook-cert",
808+
VolumeSource: corev1.VolumeSource{
809+
Secret: &corev1.SecretVolumeSource{
810+
SecretName: "test-service-cert",
811+
Items: []corev1.KeyToPath{
812+
{
813+
Key: "tls.crt",
814+
Path: "tls.crt",
815+
},
816+
{
817+
Key: "tls.key",
818+
Path: "tls.key",
819+
},
820+
},
821+
},
822+
},
823+
},
824+
},
825+
},
826+
},
827+
},
828+
},
590829
}
591830
for _, tt := range tests {
592831
t.Run(tt.name, func(t *testing.T) {

‎pkg/controller/operators/olm/apiservices.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ func (a *Operator) checkAPIServiceResources(csv *v1alpha1.ClusterServiceVersion,
110110
secretName := install.SecretName(serviceName)
111111
secret, err := a.lister.CoreV1().SecretLister().Secrets(csv.GetNamespace()).Get(secretName)
112112
if err != nil {
113-
logger.WithField("secret", secretName).Warnf("could not retrieve generated Secret")
113+
logger.WithField("secret", secretName).Warnf("could not retrieve generated Secret: %v", err)
114114
errs = append(errs, err)
115115
continue
116116
}

‎pkg/controller/operators/olm/operator.go

+3-1
Original file line numberDiff line numberDiff line change
@@ -303,7 +303,9 @@ func newOperatorWithConfig(ctx context.Context, config *operatorConfig) (*Operat
303303
}
304304

305305
// Register Secret QueueInformer
306-
secretInformer := k8sInformerFactory.Core().V1().Secrets()
306+
secretInformer := informers.NewSharedInformerFactoryWithOptions(op.opClient.KubernetesInterface(), config.resyncPeriod(), informers.WithNamespace(namespace), informers.WithTweakListOptions(func(options *metav1.ListOptions) {
307+
options.LabelSelector = labels.SelectorFromValidatedSet(map[string]string{install.OLMManagedLabelKey: install.OLMManagedLabelValue}).String()
308+
})).Core().V1().Secrets()
307309
op.lister.CoreV1().RegisterSecretLister(namespace, secretInformer.Lister())
308310
secretQueueInformer, err := queueinformer.NewQueueInformer(
309311
ctx,

‎pkg/controller/operators/olm/operator_test.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -1460,9 +1460,9 @@ func TestTransitionCSV(t *testing.T) {
14601460
deployment("a1", namespace, "sa", addAnnotations(defaultTemplateAnnotations, map[string]string{
14611461
install.OLMCAHashAnnotationKey: validCAHash,
14621462
})),
1463-
withAnnotations(keyPairToTLSSecret("a1-service-cert", namespace, signedServingPair(time.Now().Add(24*time.Hour), validCA, []string{"a1-service.ns", "a1-service.ns.svc"})), map[string]string{
1463+
withLabels(withAnnotations(keyPairToTLSSecret("a1-service-cert", namespace, signedServingPair(time.Now().Add(24*time.Hour), validCA, []string{"a1-service.ns", "a1-service.ns.svc"})), map[string]string{
14641464
install.OLMCAHashAnnotationKey: validCAHash,
1465-
}),
1465+
}), map[string]string{install.OLMManagedLabelKey: install.OLMManagedLabelValue}),
14661466
service("a1-service", namespace, "a1", 80),
14671467
serviceAccount("sa", namespace),
14681468
role("a1-service-cert", namespace, []rbacv1.PolicyRule{

0 commit comments

Comments
 (0)