Skip to content

Commit e867f21

Browse files
deads2kbertinatto
authored andcommitted
UPSTREAM: <carry>: make the PSA workload admission warnings honor the changes that SCC will eventually make to the pod
UPSTREAM: <carry>: pod-security: don't fail on SCC admission error If we propagate SCC admission error during pod extraction to PodSecurity admission, the latter will log the error instead of continuing with unmutated pod spec, and so we will not get a validation error in either the audit logs or as a warning. OpenShift-Rebase-Source: 6fe5c8f OpenShift-Rebase-Source: b4e019f UPSTREAM: <carry>: SCC pod extractor: assume default SA if SA is empty
1 parent 5ca0b65 commit e867f21

File tree

4 files changed

+125
-1
lines changed

4 files changed

+125
-1
lines changed

openshift-kube-apiserver/enablement/intialization.go

+7
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,12 @@ import (
44
"io/ioutil"
55
"path"
66

7+
"k8s.io/kubernetes/plugin/pkg/admission/security/podsecurity"
8+
79
configv1 "github.com/openshift/api/config/v1"
810
kubecontrolplanev1 "github.com/openshift/api/kubecontrolplane/v1"
911
osinv1 "github.com/openshift/api/osin/v1"
12+
"github.com/openshift/apiserver-library-go/pkg/securitycontextconstraints/sccadmission"
1013
"github.com/openshift/library-go/pkg/config/helpers"
1114
"k8s.io/apimachinery/pkg/runtime"
1215
"k8s.io/apimachinery/pkg/runtime/schema"
@@ -74,6 +77,8 @@ func ForceGlobalInitializationForOpenShift() {
7477
},
7578
})
7679

80+
podsecurity.SCCMutatingPodSpecExtractorInstance.SetSCCAdmission(SCCAdmissionPlugin)
81+
7782
// add permissions we require on our kube-apiserver
7883
// TODO, we should scrub these out
7984
bootstrappolicy.ClusterRoles = bootstrappolicy.OpenshiftClusterRoles
@@ -83,3 +88,5 @@ func ForceGlobalInitializationForOpenShift() {
8388
// SkipSystemMastersAuthorizer disable implicitly added system/master authz, and turn it into another authz mode "SystemMasters", to be added via authorization-mode
8489
authorizer.SkipSystemMastersAuthorizer()
8590
}
91+
92+
var SCCAdmissionPlugin = sccadmission.NewConstraint()

openshift-kube-apiserver/openshiftkubeapiserver/patch.go

+6
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,12 @@ func OpenShiftKubeAPIServerConfigPatch(genericConfig *genericapiserver.Config, k
8080
managementcpusoverride.NewInitializer(openshiftInformers.getOpenshiftInfraInformers().Config().V1().Infrastructures()),
8181
managednode.NewInitializer(openshiftInformers.getOpenshiftInfraInformers().Config().V1().Infrastructures()),
8282
)
83+
84+
// This is needed in order to have the correct initializers for the SCC admission plugin which is used to mutate
85+
// PodSpecs for PodSpec-y workload objects in the pod security admission plugin.
86+
enablement.SCCAdmissionPlugin.SetAuthorizer(genericConfig.Authorization.Authorizer)
87+
enablement.SCCAdmissionPlugin.SetSecurityInformers(openshiftInformers.getOpenshiftSecurityInformers().Security().V1().SecurityContextConstraints())
88+
enablement.SCCAdmissionPlugin.SetExternalKubeInformerFactory(kubeInformers)
8389
// END ADMISSION
8490

8591
// HANDLER CHAIN (with oauth server and web console)

plugin/pkg/admission/security/podsecurity/admission.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ func newPlugin(reader io.Reader) (*Plugin, error) {
115115
Configuration: config,
116116
Evaluator: evaluator,
117117
Metrics: getDefaultRecorder(),
118-
PodSpecExtractor: podsecurityadmission.DefaultPodSpecExtractor{},
118+
PodSpecExtractor: SCCMutatingPodSpecExtractorInstance,
119119
},
120120
}, nil
121121
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
package podsecurity
2+
3+
import (
4+
"context"
5+
"fmt"
6+
7+
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
8+
9+
corev1 "k8s.io/api/core/v1"
10+
"k8s.io/apimachinery/pkg/api/meta"
11+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
12+
"k8s.io/apimachinery/pkg/runtime"
13+
"k8s.io/apimachinery/pkg/runtime/schema"
14+
"k8s.io/apiserver/pkg/admission"
15+
"k8s.io/apiserver/pkg/authentication/serviceaccount"
16+
"k8s.io/apiserver/pkg/authentication/user"
17+
"k8s.io/klog/v2"
18+
"k8s.io/kubernetes/pkg/apis/core"
19+
v1 "k8s.io/kubernetes/pkg/apis/core/v1"
20+
saadmission "k8s.io/kubernetes/plugin/pkg/admission/serviceaccount"
21+
podsecurityadmission "k8s.io/pod-security-admission/admission"
22+
)
23+
24+
type SCCMutatingPodSpecExtractor struct {
25+
sccAdmission admission.MutationInterface
26+
delegate podsecurityadmission.PodSpecExtractor
27+
}
28+
29+
var SCCMutatingPodSpecExtractorInstance = &SCCMutatingPodSpecExtractor{
30+
delegate: podsecurityadmission.DefaultPodSpecExtractor{},
31+
}
32+
33+
func (s *SCCMutatingPodSpecExtractor) SetSCCAdmission(sccAdmission admission.MutationInterface) {
34+
s.sccAdmission = sccAdmission
35+
}
36+
37+
func (s *SCCMutatingPodSpecExtractor) HasPodSpec(gr schema.GroupResource) bool {
38+
return s.delegate.HasPodSpec(gr)
39+
}
40+
41+
func (s *SCCMutatingPodSpecExtractor) ExtractPodSpec(obj runtime.Object) (*metav1.ObjectMeta, *corev1.PodSpec, error) {
42+
if s.sccAdmission == nil {
43+
return s.delegate.ExtractPodSpec(obj)
44+
}
45+
46+
switch obj := obj.(type) {
47+
case *corev1.Pod:
48+
return s.delegate.ExtractPodSpec(obj)
49+
}
50+
51+
podTemplateMeta, originalPodSpec, err := s.delegate.ExtractPodSpec(obj)
52+
if err != nil {
53+
return podTemplateMeta, originalPodSpec, err
54+
}
55+
if originalPodSpec == nil {
56+
return nil, nil, nil
57+
}
58+
objectMeta, err := meta.Accessor(obj)
59+
if err != nil {
60+
return podTemplateMeta, originalPodSpec, fmt.Errorf("unable to get metadata for SCC mutation: %w", err)
61+
}
62+
63+
pod := &corev1.Pod{
64+
ObjectMeta: *podTemplateMeta.DeepCopy(),
65+
Spec: *originalPodSpec.DeepCopy(),
66+
}
67+
if len(pod.Namespace) == 0 {
68+
pod.Namespace = objectMeta.GetNamespace()
69+
}
70+
if len(pod.Name) == 0 {
71+
pod.Name = "pod-for-container-named-" + objectMeta.GetName()
72+
}
73+
if len(pod.Spec.ServiceAccountName) == 0 {
74+
pod.Spec.ServiceAccountName = saadmission.DefaultServiceAccountName
75+
}
76+
internalPod := &core.Pod{}
77+
if err := v1.Convert_v1_Pod_To_core_Pod(pod, internalPod, nil); err != nil {
78+
return nil, nil, err
79+
}
80+
81+
admissionAttributes := admission.NewAttributesRecord(
82+
internalPod,
83+
nil,
84+
corev1.SchemeGroupVersion.WithKind("Pod"),
85+
pod.Namespace,
86+
pod.Name,
87+
corev1.SchemeGroupVersion.WithResource("pods"),
88+
"",
89+
admission.Create,
90+
nil,
91+
false,
92+
&user.DefaultInfo{
93+
Name: serviceaccount.MakeUsername(pod.Namespace, pod.Spec.ServiceAccountName),
94+
UID: "",
95+
Groups: append([]string{user.AllAuthenticated}, serviceaccount.MakeGroupNames(pod.Namespace)...),
96+
Extra: nil,
97+
})
98+
if err := s.sccAdmission.Admit(context.Background(), admissionAttributes, nil); err != nil {
99+
// don't fail the request, just warn if SCC will fail
100+
klog.ErrorS(err, "failed to mutate object for PSA using SCC")
101+
utilruntime.HandleError(fmt.Errorf("failed to mutate object for PSA using SCC: %w", err))
102+
// TODO remove this failure we're causing when SCC fails, but for now we actually need to see our test fail because that was almost really bad.
103+
return podTemplateMeta, originalPodSpec, nil
104+
}
105+
106+
if err := v1.Convert_core_Pod_To_v1_Pod(internalPod, pod, nil); err != nil {
107+
return nil, nil, err
108+
}
109+
110+
return podTemplateMeta, &pod.Spec, nil
111+
}

0 commit comments

Comments
 (0)