Skip to content

Commit

Permalink
Sec revert (#2808)
Browse files Browse the repository at this point in the history 
* Revert "Unpack job security updates (#2805)"

This reverts commit e568cde.

Signed-off-by: perdasilva <perdasilva@redhat.com>

* Revert "Update unpack job pod security (#2793)"

This reverts commit eedad28.

Signed-off-by: perdasilva <perdasilva@redhat.com>

* Revert "Update CatalogSource Pod security context (#2782)"

This reverts commit 99b51e7.

Signed-off-by: perdasilva <perdasilva@redhat.com>
  • Loading branch information
@perdasilva
perdasilva authored Jun 23, 2022
1 parent e568cde commit 0b7970c
Show file tree
Hide file tree
Showing 40 changed files with 382 additions and 2,239 deletions.
2 changes: 1 addition & 1 deletion Dockerfile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ FROM quay.io/fedora/fedora:34-x86_64 as builder
LABEL stage=builder
WORKDIR /build

# install dependencies and go 1.17
# install dependencies and go 1.16

# copy just enough of the git repo to parse HEAD, used to record version in OLM binaries
RUN dnf update -y && dnf install -y bash make git mercurial jq wget && dnf upgrade -y
Expand Down
66 changes: 21 additions & 45 deletions pkg/controller/bundle/bundle_unpacker.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import (
listersoperatorsv1alpha1 "github.com/operator-framework/operator-lifecycle-manager/pkg/api/client/listers/operators/v1alpha1"
"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/install"
"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/resolver/projection"
"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/security"
)

const (
Expand Down Expand Up @@ -191,10 +190,6 @@ func (c *ConfigMapUnpacker) job(cmRef *corev1.ObjectReference, bundlePath string
},
},
}

// Apply Pod security
security.ApplyPodSpecSecurity(&job.Spec.Template.Spec)

job.SetNamespace(cmRef.Namespace)
job.SetName(cmRef.Name)
job.SetOwnerReferences([]metav1.OwnerReference{ownerRef(cmRef)})
Expand Down Expand Up @@ -435,7 +430,7 @@ func (c *ConfigMapUnpacker) UnpackBundle(lookup *operatorsv1alpha1.BundleLookup,
return
}

_, err = c.ensureRole(cmRef, c.getRolePolicyRules(cmRef))
_, err = c.ensureRole(cmRef)
if err != nil {
return
}
Expand Down Expand Up @@ -610,13 +605,27 @@ func (c *ConfigMapUnpacker) ensureJob(cmRef *corev1.ObjectReference, bundlePath
return
}

func (c *ConfigMapUnpacker) ensureRole(cmRef *corev1.ObjectReference, policyRules []rbacv1.PolicyRule) (role *rbacv1.Role, err error) {
func (c *ConfigMapUnpacker) ensureRole(cmRef *corev1.ObjectReference) (role *rbacv1.Role, err error) {
if cmRef == nil {
return nil, fmt.Errorf("configmap reference is nil")
}

rule := rbacv1.PolicyRule{
APIGroups: []string{
"",
},
Verbs: []string{
"create", "get", "update",
},
Resources: []string{
"configmaps",
},
ResourceNames: []string{
cmRef.Name,
},
}
fresh := &rbacv1.Role{
Rules: policyRules,
Rules: []rbacv1.PolicyRule{rule},
}
fresh.SetNamespace(cmRef.Namespace)
fresh.SetName(cmRef.Name)
Expand All @@ -632,43 +641,19 @@ func (c *ConfigMapUnpacker) ensureRole(cmRef *corev1.ObjectReference, policyRule
}

// Add the policy rule if necessary
var ruleDiff []rbacv1.PolicyRule
for _, proposed := range policyRules {
if !containsRule(role.Rules, proposed) {
ruleDiff = append(ruleDiff, proposed)
for _, existing := range role.Rules {
if equality.Semantic.DeepDerivative(rule, existing) {
return
}
}

role = role.DeepCopy()
role.Rules = append(role.Rules, ruleDiff...)
role.Rules = append(role.Rules, rule)

role, err = c.client.RbacV1().Roles(role.GetNamespace()).Update(context.TODO(), role, metav1.UpdateOptions{})

return
}

// getRolePolicyRules returns the set of policy rules used by the role attached to the
// bundle unpacker service account. This method lends itself to easier downstream patching when additional
// policy rules are required, e.g. for Openshift SCC
func (c *ConfigMapUnpacker) getRolePolicyRules(cmRef *corev1.ObjectReference) []rbacv1.PolicyRule {
return []rbacv1.PolicyRule{
{
APIGroups: []string{
"",
},
Verbs: []string{
"get", "update",
},
Resources: []string{
"configmaps",
},
ResourceNames: []string{
cmRef.Name,
},
},
}
}

func (c *ConfigMapUnpacker) ensureRoleBinding(cmRef *corev1.ObjectReference) (roleBinding *rbacv1.RoleBinding, err error) {
fresh := &rbacv1.RoleBinding{
Subjects: []rbacv1.Subject{
Expand Down Expand Up @@ -748,12 +733,3 @@ func getCondition(job *batchv1.Job, conditionType batchv1.JobConditionType) (con
}
return
}

func containsRule(rules []rbacv1.PolicyRule, rule rbacv1.PolicyRule) bool {
for _, r := range rules {
if equality.Semantic.DeepDerivative(r, rule) {
return true
}
}
return false
}
61 changes: 7 additions & 54 deletions pkg/controller/bundle/bundle_unpacker_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,29 +68,6 @@ func TestConfigMapUnpacker(t *testing.T) {
roleBindings []*rbacv1.RoleBinding
}

var expectedReadOnlyRootFilesystem = false
var expectedAllowPrivilegeEscalation = false
var expectedRunAsNonRoot = true
var expectedRunAsUser int64 = 1001
var expectedPrivileged = false

var expectedContainerSecurityContext = &corev1.SecurityContext{
Privileged: &expectedPrivileged,
ReadOnlyRootFilesystem: &expectedReadOnlyRootFilesystem,
AllowPrivilegeEscalation: &expectedAllowPrivilegeEscalation,
Capabilities: &corev1.Capabilities{
Drop: []corev1.Capability{"ALL"},
},
}

var expectedPodSecurityContext = &corev1.PodSecurityContext{
RunAsNonRoot: &expectedRunAsNonRoot,
RunAsUser: &expectedRunAsUser,
SeccompProfile: &corev1.SeccompProfile{
Type: corev1.SeccompProfileTypeRuntimeDefault,
},
}

tests := []struct {
description string
fields fields
Expand Down Expand Up @@ -243,7 +220,6 @@ func TestConfigMapUnpacker(t *testing.T) {
Spec: corev1.PodSpec{
RestartPolicy: corev1.RestartPolicyNever,
ImagePullSecrets: []corev1.LocalObjectReference{{Name: "my-secret"}},
SecurityContext: expectedPodSecurityContext,
Containers: []corev1.Container{
{
Name: "extract",
Expand All @@ -267,7 +243,6 @@ func TestConfigMapUnpacker(t *testing.T) {
corev1.ResourceMemory: resource.MustParse("50Mi"),
},
},
SecurityContext: expectedContainerSecurityContext,
},
},
InitContainers: []corev1.Container{
Expand All @@ -287,7 +262,6 @@ func TestConfigMapUnpacker(t *testing.T) {
corev1.ResourceMemory: resource.MustParse("50Mi"),
},
},
SecurityContext: expectedContainerSecurityContext,
},
{
Name: "pull",
Expand All @@ -310,7 +284,6 @@ func TestConfigMapUnpacker(t *testing.T) {
corev1.ResourceMemory: resource.MustParse("50Mi"),
},
},
SecurityContext: expectedContainerSecurityContext,
},
},
Volumes: []corev1.Volume{
Expand Down Expand Up @@ -353,7 +326,7 @@ func TestConfigMapUnpacker(t *testing.T) {
"",
},
Verbs: []string{
"get", "update",
"create", "get", "update",
},
Resources: []string{
"configmaps",
Expand Down Expand Up @@ -423,8 +396,7 @@ func TestConfigMapUnpacker(t *testing.T) {
Name: pathHash,
},
Spec: corev1.PodSpec{
RestartPolicy: corev1.RestartPolicyNever,
SecurityContext: expectedPodSecurityContext,
RestartPolicy: corev1.RestartPolicyNever,
Containers: []corev1.Container{
{
Name: "extract",
Expand All @@ -448,7 +420,6 @@ func TestConfigMapUnpacker(t *testing.T) {
corev1.ResourceMemory: resource.MustParse("50Mi"),
},
},
SecurityContext: expectedContainerSecurityContext,
},
},
InitContainers: []corev1.Container{
Expand All @@ -468,7 +439,6 @@ func TestConfigMapUnpacker(t *testing.T) {
corev1.ResourceMemory: resource.MustParse("50Mi"),
},
},
SecurityContext: expectedContainerSecurityContext,
},
{
Name: "pull",
Expand All @@ -491,7 +461,6 @@ func TestConfigMapUnpacker(t *testing.T) {
corev1.ResourceMemory: resource.MustParse("50Mi"),
},
},
SecurityContext: expectedContainerSecurityContext,
},
},
Volumes: []corev1.Volume{
Expand Down Expand Up @@ -645,8 +614,7 @@ func TestConfigMapUnpacker(t *testing.T) {
Name: pathHash,
},
Spec: corev1.PodSpec{
RestartPolicy: corev1.RestartPolicyNever,
SecurityContext: expectedPodSecurityContext,
RestartPolicy: corev1.RestartPolicyNever,
Containers: []corev1.Container{
{
Name: "extract",
Expand All @@ -670,7 +638,6 @@ func TestConfigMapUnpacker(t *testing.T) {
corev1.ResourceMemory: resource.MustParse("50Mi"),
},
},
SecurityContext: expectedContainerSecurityContext,
},
},
InitContainers: []corev1.Container{
Expand All @@ -690,7 +657,6 @@ func TestConfigMapUnpacker(t *testing.T) {
corev1.ResourceMemory: resource.MustParse("50Mi"),
},
},
SecurityContext: expectedContainerSecurityContext,
},
{
Name: "pull",
Expand All @@ -713,7 +679,6 @@ func TestConfigMapUnpacker(t *testing.T) {
corev1.ResourceMemory: resource.MustParse("50Mi"),
},
},
SecurityContext: expectedContainerSecurityContext,
},
},
Volumes: []corev1.Volume{
Expand Down Expand Up @@ -769,7 +734,7 @@ func TestConfigMapUnpacker(t *testing.T) {
"",
},
Verbs: []string{
"get", "update",
"create", "get", "update",
},
Resources: []string{
"configmaps",
Expand Down Expand Up @@ -861,8 +826,7 @@ func TestConfigMapUnpacker(t *testing.T) {
Name: pathHash,
},
Spec: corev1.PodSpec{
RestartPolicy: corev1.RestartPolicyNever,
SecurityContext: expectedPodSecurityContext,
RestartPolicy: corev1.RestartPolicyNever,
Containers: []corev1.Container{
{
Name: "extract",
Expand All @@ -886,7 +850,6 @@ func TestConfigMapUnpacker(t *testing.T) {
corev1.ResourceMemory: resource.MustParse("50Mi"),
},
},
SecurityContext: expectedContainerSecurityContext,
},
},
InitContainers: []corev1.Container{
Expand All @@ -906,7 +869,6 @@ func TestConfigMapUnpacker(t *testing.T) {
corev1.ResourceMemory: resource.MustParse("50Mi"),
},
},
SecurityContext: expectedContainerSecurityContext,
},
{
Name: "pull",
Expand All @@ -929,7 +891,6 @@ func TestConfigMapUnpacker(t *testing.T) {
corev1.ResourceMemory: resource.MustParse("50Mi"),
},
},
SecurityContext: expectedContainerSecurityContext,
},
},
Volumes: []corev1.Volume{
Expand Down Expand Up @@ -1047,8 +1008,7 @@ func TestConfigMapUnpacker(t *testing.T) {
Name: pathHash,
},
Spec: corev1.PodSpec{
RestartPolicy: corev1.RestartPolicyNever,
SecurityContext: expectedPodSecurityContext,
RestartPolicy: corev1.RestartPolicyNever,
Containers: []corev1.Container{
{
Name: "extract",
Expand All @@ -1072,7 +1032,6 @@ func TestConfigMapUnpacker(t *testing.T) {
corev1.ResourceMemory: resource.MustParse("50Mi"),
},
},
SecurityContext: expectedContainerSecurityContext,
},
},
InitContainers: []corev1.Container{
Expand All @@ -1092,7 +1051,6 @@ func TestConfigMapUnpacker(t *testing.T) {
corev1.ResourceMemory: resource.MustParse("50Mi"),
},
},
SecurityContext: expectedContainerSecurityContext,
},
{
Name: "pull",
Expand All @@ -1115,7 +1073,6 @@ func TestConfigMapUnpacker(t *testing.T) {
corev1.ResourceMemory: resource.MustParse("50Mi"),
},
},
SecurityContext: expectedContainerSecurityContext,
},
},
Volumes: []corev1.Volume{
Expand Down Expand Up @@ -1244,8 +1201,7 @@ func TestConfigMapUnpacker(t *testing.T) {
Name: pathHash,
},
Spec: corev1.PodSpec{
RestartPolicy: corev1.RestartPolicyNever,
SecurityContext: expectedPodSecurityContext,
RestartPolicy: corev1.RestartPolicyNever,
Containers: []corev1.Container{
{
Name: "extract",
Expand All @@ -1269,7 +1225,6 @@ func TestConfigMapUnpacker(t *testing.T) {
corev1.ResourceMemory: resource.MustParse("50Mi"),
},
},
SecurityContext: expectedContainerSecurityContext,
},
},
InitContainers: []corev1.Container{
Expand All @@ -1289,7 +1244,6 @@ func TestConfigMapUnpacker(t *testing.T) {
corev1.ResourceMemory: resource.MustParse("50Mi"),
},
},
SecurityContext: expectedContainerSecurityContext,
},
{
Name: "pull",
Expand All @@ -1312,7 +1266,6 @@ func TestConfigMapUnpacker(t *testing.T) {
corev1.ResourceMemory: resource.MustParse("50Mi"),
},
},
SecurityContext: expectedContainerSecurityContext,
},
},
Volumes: []corev1.Volume{
Expand Down
Loading

0 comments on commit 0b7970c

Please sign in to comment.