-
Notifications
You must be signed in to change notification settings - Fork 545
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
OCPBUGS-17157: pkg/controller: label RBAC with content hash #3034
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -108,7 +108,6 @@ type Operator struct { | |
client versioned.Interface | ||
dynamicClient dynamic.Interface | ||
lister operatorlister.OperatorLister | ||
k8sLabelQueueSets map[schema.GroupVersionResource]workqueue.RateLimitingInterface | ||
catsrcQueueSet *queueinformer.ResourceQueueSet | ||
subQueueSet *queueinformer.ResourceQueueSet | ||
ipQueueSet *queueinformer.ResourceQueueSet | ||
|
@@ -202,7 +201,6 @@ func NewOperator(ctx context.Context, kubeconfigPath string, clock utilclock.Clo | |
lister: lister, | ||
namespace: operatorNamespace, | ||
recorder: eventRecorder, | ||
k8sLabelQueueSets: map[schema.GroupVersionResource]workqueue.RateLimitingInterface{}, | ||
catsrcQueueSet: queueinformer.NewEmptyResourceQueueSet(), | ||
subQueueSet: queueinformer.NewEmptyResourceQueueSet(), | ||
ipQueueSet: queueinformer.NewEmptyResourceQueueSet(), | ||
|
@@ -386,11 +384,12 @@ func NewOperator(ctx context.Context, kubeconfigPath string, clock utilclock.Clo | |
if canFilter { | ||
return nil | ||
} | ||
op.k8sLabelQueueSets[gvr] = workqueue.NewRateLimitingQueueWithConfig(workqueue.DefaultControllerRateLimiter(), workqueue.RateLimitingQueueConfig{ | ||
queue := workqueue.NewRateLimitingQueueWithConfig(workqueue.DefaultControllerRateLimiter(), workqueue.RateLimitingQueueConfig{ | ||
Name: gvr.String(), | ||
}) | ||
queueInformer, err := queueinformer.NewQueueInformer( | ||
ctx, | ||
queueinformer.WithQueue(queue), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I forgot this in the original PR to add the labeler functionality. |
||
queueinformer.WithLogger(op.logger), | ||
queueinformer.WithInformer(informer), | ||
queueinformer.WithSyncer(sync.ToSyncer()), | ||
|
@@ -416,6 +415,18 @@ func NewOperator(ctx context.Context, kubeconfigPath string, clock utilclock.Clo | |
)); err != nil { | ||
return nil, err | ||
} | ||
if err := labelObjects(rolesgvk, roleInformer.Informer(), labeller.ContentHashLabeler[*rbacv1.Role, *rbacv1applyconfigurations.RoleApplyConfiguration]( | ||
ctx, op.logger, labeller.ContentHashFilter, | ||
func(role *rbacv1.Role) (string, error) { | ||
return resolver.PolicyRuleHashLabelValue(role.Rules) | ||
}, | ||
rbacv1applyconfigurations.Role, | ||
func(namespace string, ctx context.Context, cfg *rbacv1applyconfigurations.RoleApplyConfiguration, opts metav1.ApplyOptions) (*rbacv1.Role, error) { | ||
return op.opClient.KubernetesInterface().RbacV1().Roles(namespace).Apply(ctx, cfg, opts) | ||
}, | ||
)); err != nil { | ||
return nil, err | ||
} | ||
|
||
// Wire RoleBindings | ||
roleBindingInformer := k8sInformerFactory.Rbac().V1().RoleBindings() | ||
|
@@ -432,6 +443,18 @@ func NewOperator(ctx context.Context, kubeconfigPath string, clock utilclock.Clo | |
)); err != nil { | ||
return nil, err | ||
} | ||
if err := labelObjects(rolebindingsgvk, roleBindingInformer.Informer(), labeller.ContentHashLabeler[*rbacv1.RoleBinding, *rbacv1applyconfigurations.RoleBindingApplyConfiguration]( | ||
ctx, op.logger, labeller.ContentHashFilter, | ||
func(roleBinding *rbacv1.RoleBinding) (string, error) { | ||
return resolver.RoleReferenceAndSubjectHashLabelValue(roleBinding.RoleRef, roleBinding.Subjects) | ||
}, | ||
rbacv1applyconfigurations.RoleBinding, | ||
func(namespace string, ctx context.Context, cfg *rbacv1applyconfigurations.RoleBindingApplyConfiguration, opts metav1.ApplyOptions) (*rbacv1.RoleBinding, error) { | ||
return op.opClient.KubernetesInterface().RbacV1().RoleBindings(namespace).Apply(ctx, cfg, opts) | ||
}, | ||
)); err != nil { | ||
return nil, err | ||
} | ||
|
||
// Wire ServiceAccounts | ||
serviceAccountInformer := k8sInformerFactory.Core().V1().ServiceAccounts() | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,56 @@ | ||
package labeller | ||
|
||
import ( | ||
"context" | ||
"fmt" | ||
|
||
"github.com/operator-framework/api/pkg/operators/v1alpha1" | ||
"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/resolver" | ||
"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/ownerutil" | ||
"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/queueinformer" | ||
"github.com/sirupsen/logrus" | ||
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
) | ||
|
||
func hasHashLabel(obj metav1.Object) bool { | ||
_, ok := obj.GetLabels()[resolver.ContentHashLabelKey] | ||
return ok | ||
} | ||
|
||
func ContentHashLabeler[T metav1.Object, A ApplyConfig[A]]( | ||
ctx context.Context, | ||
logger *logrus.Logger, | ||
check func(metav1.Object) bool, | ||
hasher func(object T) (string, error), | ||
applyConfigFor func(name, namespace string) A, | ||
apply func(namespace string, ctx context.Context, cfg A, opts metav1.ApplyOptions) (T, error), | ||
) queueinformer.LegacySyncHandler { | ||
return func(obj interface{}) error { | ||
cast, ok := obj.(T) | ||
if !ok { | ||
err := fmt.Errorf("wrong type %T, expected %T: %#v", obj, new(T), obj) | ||
logger.WithError(err).Error("casting failed") | ||
return fmt.Errorf("casting failed: %w", err) | ||
} | ||
|
||
if _, _, ok := ownerutil.GetOwnerByKindLabel(cast, v1alpha1.ClusterServiceVersionKind); !ok { | ||
return nil | ||
} | ||
|
||
if !check(cast) || hasHashLabel(cast) { | ||
return nil | ||
} | ||
|
||
hash, err := hasher(cast) | ||
if err != nil { | ||
return fmt.Errorf("failed to calculate hash: %w", err) | ||
} | ||
|
||
cfg := applyConfigFor(cast.GetName(), cast.GetNamespace()) | ||
cfg.WithLabels(map[string]string{ | ||
resolver.ContentHashLabelKey: hash, | ||
}) | ||
_, err = apply(cast.GetNamespace(), ctx, cfg, metav1.ApplyOptions{}) | ||
return err | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,11 @@ | ||
package resolver | ||
|
||
import ( | ||
"crypto/sha256" | ||
"encoding/json" | ||
"fmt" | ||
"hash/fnv" | ||
"math/big" | ||
|
||
corev1 "k8s.io/api/core/v1" | ||
rbacv1 "k8s.io/api/rbac/v1" | ||
|
@@ -62,6 +65,37 @@ func (o *OperatorPermissions) AddClusterRoleBinding(clusterRoleBinding *rbacv1.C | |
o.ClusterRoleBindings = append(o.ClusterRoleBindings, clusterRoleBinding) | ||
} | ||
|
||
const ContentHashLabelKey = "olm.permissions.hash" | ||
|
||
func PolicyRuleHashLabelValue(rules []rbacv1.PolicyRule) (string, error) { | ||
raw, err := json.Marshal(rules) | ||
if err != nil { | ||
return "", err | ||
} | ||
return toBase62(sha256.Sum224(raw)), nil | ||
} | ||
Comment on lines
+70
to
+76
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It seems like this could cause an issue if two operators define the same rules for a clusterRole. |
||
|
||
func RoleReferenceAndSubjectHashLabelValue(roleRef rbacv1.RoleRef, subjects []rbacv1.Subject) (string, error) { | ||
var container = struct { | ||
RoleRef rbacv1.RoleRef | ||
Subjects []rbacv1.Subject | ||
}{ | ||
RoleRef: roleRef, | ||
Subjects: subjects, | ||
} | ||
raw, err := json.Marshal(&container) | ||
if err != nil { | ||
return "", err | ||
} | ||
return toBase62(sha256.Sum224(raw)), nil | ||
} | ||
|
||
func toBase62(hash [28]byte) string { | ||
var i big.Int | ||
i.SetBytes(hash[:]) | ||
return i.Text(62) | ||
} | ||
|
||
func RBACForClusterServiceVersion(csv *v1alpha1.ClusterServiceVersion) (map[string]*OperatorPermissions, error) { | ||
permissions := map[string]*OperatorPermissions{} | ||
|
||
|
@@ -100,6 +134,11 @@ func RBACForClusterServiceVersion(csv *v1alpha1.ClusterServiceVersion) (map[stri | |
}, | ||
Rules: permission.Rules, | ||
} | ||
hash, err := PolicyRuleHashLabelValue(permission.Rules) | ||
if err != nil { | ||
return nil, fmt.Errorf("failed to hash permission rules: %w", err) | ||
} | ||
role.Labels[ContentHashLabelKey] = hash | ||
permissions[permission.ServiceAccountName].AddRole(role) | ||
|
||
// Create RoleBinding | ||
|
@@ -120,6 +159,11 @@ func RBACForClusterServiceVersion(csv *v1alpha1.ClusterServiceVersion) (map[stri | |
Namespace: csv.GetNamespace(), | ||
}}, | ||
} | ||
hash, err = RoleReferenceAndSubjectHashLabelValue(roleBinding.RoleRef, roleBinding.Subjects) | ||
if err != nil { | ||
return nil, fmt.Errorf("failed to hash binding content: %w", err) | ||
} | ||
roleBinding.Labels[ContentHashLabelKey] = hash | ||
permissions[permission.ServiceAccountName].AddRoleBinding(roleBinding) | ||
} | ||
|
||
|
@@ -142,6 +186,11 @@ func RBACForClusterServiceVersion(csv *v1alpha1.ClusterServiceVersion) (map[stri | |
}, | ||
Rules: permission.Rules, | ||
} | ||
hash, err := PolicyRuleHashLabelValue(permission.Rules) | ||
if err != nil { | ||
return nil, fmt.Errorf("failed to hash permission rules: %w", err) | ||
} | ||
role.Labels[ContentHashLabelKey] = hash | ||
permissions[permission.ServiceAccountName].AddClusterRole(role) | ||
|
||
// Create ClusterRoleBinding | ||
|
@@ -162,6 +211,11 @@ func RBACForClusterServiceVersion(csv *v1alpha1.ClusterServiceVersion) (map[stri | |
Namespace: csv.GetNamespace(), | ||
}}, | ||
} | ||
hash, err = RoleReferenceAndSubjectHashLabelValue(roleBinding.RoleRef, roleBinding.Subjects) | ||
if err != nil { | ||
return nil, fmt.Errorf("failed to hash binding content: %w", err) | ||
} | ||
roleBinding.Labels[ContentHashLabelKey] = hash | ||
permissions[permission.ServiceAccountName].AddClusterRoleBinding(roleBinding) | ||
} | ||
return permissions, nil | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added this in a previous commit but was in a copy-pasta mode and it's not needed.