github/kubernetes
4
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge pull request #118356 from ritazh/ec-admission

Browse Source 
Add ephemeralcontainer to imagepolicy securityaccount admission plugin
This commit is contained in:
Kubernetes Prow Robot
2023-06-03 05:30:41 -07:00
committed by GitHub
parent 039ae1edf5 d6168bb658
commit bb87860868
4 changed files with 310 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
plugin/pkg/admission/imagepolicy/admission.go
View File

@@ -46,6 +46,7 @@ import (
// PluginName indicates name of admission plugin.
const PluginName = "ImagePolicyWebhook"
const ephemeralcontainers = "ephemeralcontainers"
// AuditKeyPrefix is used as the prefix for all audit keys handled by this
// pluggin. Some well known suffixes are listed below.
@@ -132,8 +133,9 @@ func (a *Plugin) webhookError(pod *api.Pod, attributes admission.Attributes, err
// Validate makes an admission decision based on the request attributes
func (a *Plugin) Validate(ctx context.Context, attributes admission.Attributes, o admission.ObjectInterfaces) (err error) {
// Ignore all calls to subresources or resources other than pods.
if attributes.GetSubresource() != "" || attributes.GetResource().GroupResource() != api.Resource("pods") {
// Ignore all calls to subresources other than ephemeralcontainers or calls to resources other than pods.
subresource := attributes.GetSubresource()
if (subresource != "" && subresource != ephemeralcontainers) || attributes.GetResource().GroupResource() != api.Resource("pods") {
return nil
}
@@ -144,6 +146,7 @@ func (a *Plugin) Validate(ctx context.Context, attributes admission.Attributes,
// Build list of ImageReviewContainerSpec
var imageReviewContainerSpecs []v1alpha1.ImageReviewContainerSpec
if subresource == "" {
containers := make([]api.Container, 0, len(pod.Spec.Containers)+len(pod.Spec.InitContainers))
containers = append(containers, pod.Spec.Containers...)
containers = append(containers, pod.Spec.InitContainers...)
@@ -152,6 +155,13 @@ func (a *Plugin) Validate(ctx context.Context, attributes admission.Attributes,
Image: c.Image,
})
}
} else if subresource == ephemeralcontainers {
for _, c := range pod.Spec.EphemeralContainers {
imageReviewContainerSpecs = append(imageReviewContainerSpecs, v1alpha1.ImageReviewContainerSpec{
Image: c.Image,
})
}
}
imageReview := v1alpha1.ImageReview{
Spec: v1alpha1.ImageReviewSpec{
Containers: imageReviewContainerSpecs,

172
plugin/pkg/admission/imagepolicy/admission_test.go
View File

@@ -39,7 +39,6 @@ import (
api "k8s.io/kubernetes/pkg/apis/core"
"fmt"
"io/ioutil"
"os"
"path/filepath"
"text/template"
@@ -94,7 +93,7 @@ func TestNewFromConfig(t *testing.T) {
{data.Key, clientKey},
}
for _, file := range files {
if err := ioutil.WriteFile(file.name, file.data, 0400); err != nil {
if err := os.WriteFile(file.name, file.data, 0400); err != nil {
t.Fatal(err)
}
}
@@ -198,10 +197,11 @@ current-context: default
// Use a closure so defer statements trigger between loop iterations.
t.Run(tt.msg, func(t *testing.T) {
err := func() error {
tempfile, err := ioutil.TempFile("", "")
tempfile, err := os.CreateTemp("", "")
if err != nil {
return err
}
p := tempfile.Name()
defer utiltesting.CloseAndRemove(t, tempfile)
tmpl, err := template.New("test").Parse(tt.kubeConfigTmpl)
@@ -212,11 +212,12 @@ current-context: default
return fmt.Errorf("failed to execute test template: %v", err)
}
tempconfigfile, err := ioutil.TempFile("", "")
tempconfigfile, err := os.CreateTemp("", "")
if err != nil {
return err
}
defer os.Remove(tempconfigfile.Name())
pc := tempconfigfile.Name()
defer utiltesting.CloseAndRemove(t, tempconfigfile)
configTmpl, err := template.New("testconfig").Parse(defaultConfigTmplJSON)
if err != nil {
@@ -229,7 +230,7 @@ current-context: default
RetryBackoff int
DefaultAllow bool
}{
KubeConfig: tempfile.Name(),
KubeConfig: p,
AllowTTL: 500,
DenyTTL: 500,
RetryBackoff: 500,
@@ -240,7 +241,7 @@ current-context: default
}
// Create a new admission controller
configFile, err := os.Open(tempconfigfile.Name())
configFile, err := os.Open(pc)
if err != nil {
return fmt.Errorf("failed to read test config: %v", err)
}
@@ -358,13 +359,13 @@ func (m *mockService) HTTPStatusCode() int { return m.statusCode }
// newImagePolicyWebhook creates a temporary kubeconfig file from the provided arguments and attempts to load
// a new newImagePolicyWebhook from it.
func newImagePolicyWebhook(t *testing.T, callbackURL string, clientCert, clientKey, ca []byte, cacheTime time.Duration, defaultAllow bool) (*Plugin, error) {
tempfile, err := ioutil.TempFile("", "")
func newImagePolicyWebhook(callbackURL string, clientCert, clientKey, ca []byte, cacheTime time.Duration, defaultAllow bool) (*Plugin, error) {
tempfile, err := os.CreateTemp("", "")
if err != nil {
return nil, err
}
p := tempfile.Name()
defer utiltesting.CloseAndRemove(t, tempfile)
defer os.Remove(p)
config := v1.Config{
Clusters: []v1.NamedCluster{
{
@@ -381,12 +382,12 @@ func newImagePolicyWebhook(t *testing.T, callbackURL string, clientCert, clientK
return nil, err
}
tempconfigfile, err := ioutil.TempFile("", "")
tempconfigfile, err := os.CreateTemp("", "")
if err != nil {
return nil, err
}
pc := tempconfigfile.Name()
defer utiltesting.CloseAndRemove(t, tempconfigfile)
defer os.Remove(pc)
configTmpl, err := template.New("testconfig").Parse(defaultConfigTmplYAML)
if err != nil {
@@ -478,7 +479,7 @@ func TestTLSConfig(t *testing.T) {
}
defer server.Close()
wh, err := newImagePolicyWebhook(t, server.URL, tt.clientCert, tt.clientKey, tt.clientCA, -1, false)
wh, err := newImagePolicyWebhook(server.URL, tt.clientCert, tt.clientKey, tt.clientCA, -1, false)
if err != nil {
t.Errorf("%s: failed to create client: %v", tt.test, err)
return
@@ -559,7 +560,7 @@ func TestWebhookCache(t *testing.T) {
defer s.Close()
// Create an admission controller that caches successful responses.
wh, err := newImagePolicyWebhook(t, s.URL, clientCert, clientKey, caCert, 200, false)
wh, err := newImagePolicyWebhook(s.URL, clientCert, clientKey, caCert, 200, false)
if err != nil {
t.Fatal(err)
}
@@ -595,17 +596,23 @@ func TestContainerCombinations(t *testing.T) {
test string
pod *api.Pod
wantAllowed, wantErr bool
subresource string
operation admission.Operation
}{
{
test: "Single container allowed",
pod: goodPod("good"),
wantAllowed: true,
subresource: "",
operation: admission.Create,
},
{
test: "Single container denied",
pod: goodPod("bad"),
wantAllowed: false,
wantErr: true,
subresource: "",
operation: admission.Create,
},
{
test: "One good container, one bad",
@@ -627,6 +634,8 @@ func TestContainerCombinations(t *testing.T) {
},
wantAllowed: false,
wantErr: true,
subresource: "",
operation: admission.Create,
},
{
test: "Multiple good containers",
@@ -648,6 +657,8 @@ func TestContainerCombinations(t *testing.T) {
},
wantAllowed: true,
wantErr: false,
subresource: "",
operation: admission.Create,
},
{
test: "Multiple bad containers",
@@ -669,6 +680,8 @@ func TestContainerCombinations(t *testing.T) {
},
wantAllowed: false,
wantErr: true,
subresource: "",
operation: admission.Create,
},
{
test: "Good container, bad init container",
@@ -692,6 +705,8 @@ func TestContainerCombinations(t *testing.T) {
},
wantAllowed: false,
wantErr: true,
subresource: "",
operation: admission.Create,
},
{
test: "Bad container, good init container",
@@ -715,6 +730,8 @@ func TestContainerCombinations(t *testing.T) {
},
wantAllowed: false,
wantErr: true,
subresource: "",
operation: admission.Create,
},
{
test: "Good container, good init container",
@@ -738,6 +755,123 @@ func TestContainerCombinations(t *testing.T) {
},
wantAllowed: true,
wantErr: false,
subresource: "",
operation: admission.Create,
},
{
test: "Good container, good init container, bad ephemeral container when updating ephemeralcontainers subresource",
pod: &api.Pod{
Spec: api.PodSpec{
ServiceAccountName: "default",
SecurityContext: &api.PodSecurityContext{},
Containers: []api.Container{
{
Image: "good",
SecurityContext: &api.SecurityContext{},
},
},
InitContainers: []api.Container{
{
Image: "good",
SecurityContext: &api.SecurityContext{},
},
},
EphemeralContainers: []api.EphemeralContainer{
{
EphemeralContainerCommon: api.EphemeralContainerCommon{
Image: "bad",
SecurityContext: &api.SecurityContext{},
},
},
},
},
},
wantAllowed: false,
wantErr: true,
subresource: "ephemeralcontainers",
operation: admission.Update,
},
{
test: "Good container, good init container, bad ephemeral container when updating subresource=='' which sets initContainer and container only",
pod: &api.Pod{
Spec: api.PodSpec{
ServiceAccountName: "default",
SecurityContext: &api.PodSecurityContext{},
Containers: []api.Container{
{
Image: "good",
SecurityContext: &api.SecurityContext{},
},
},
InitContainers: []api.Container{
{
Image: "good",
SecurityContext: &api.SecurityContext{},
},
},
EphemeralContainers: []api.EphemeralContainer{
{
EphemeralContainerCommon: api.EphemeralContainerCommon{
Image: "bad",
SecurityContext: &api.SecurityContext{},
},
},
},
},
},
wantAllowed: true,
wantErr: false,
subresource: "",
operation: admission.Update,
},
{
test: "Bad container, good ephemeral container when updating subresource=='ephemeralcontainers' which sets ephemeralcontainers only",
pod: &api.Pod{
Spec: api.PodSpec{
ServiceAccountName: "default",
SecurityContext: &api.PodSecurityContext{},
Containers: []api.Container{
{
Image: "bad",
SecurityContext: &api.SecurityContext{},
},
},
EphemeralContainers: []api.EphemeralContainer{
{
EphemeralContainerCommon: api.EphemeralContainerCommon{
Image: "good",
SecurityContext: &api.SecurityContext{},
},
},
},
},
},
wantAllowed: true,
wantErr: false,
subresource: "ephemeralcontainers",
operation: admission.Update,
},
{
test: "Good ephemeral container",
pod: &api.Pod{
Spec: api.PodSpec{
ServiceAccountName: "default",
SecurityContext: &api.PodSecurityContext{},
EphemeralContainers: []api.EphemeralContainer{
{
EphemeralContainerCommon: api.EphemeralContainerCommon{
Image: "good",
SecurityContext: &api.SecurityContext{},
},
},
},
},
},
wantAllowed: true,
wantErr: false,
subresource: "ephemeralcontainers",
operation: admission.Update,
},
}
for _, tt := range tests {
@@ -753,13 +887,13 @@ func TestContainerCombinations(t *testing.T) {
}
defer server.Close()
wh, err := newImagePolicyWebhook(t, server.URL, clientCert, clientKey, caCert, 0, false)
wh, err := newImagePolicyWebhook(server.URL, clientCert, clientKey, caCert, 0, false)
if err != nil {
t.Errorf("%s: failed to create client: %v", tt.test, err)
return
}
attr := admission.NewAttributesRecord(tt.pod, nil, api.Kind("Pod").WithVersion("version"), "namespace", "", api.Resource("pods").WithVersion("version"), "", admission.Create, &metav1.CreateOptions{}, false, &user.DefaultInfo{})
attr := admission.NewAttributesRecord(tt.pod, nil, api.Kind("Pod").WithVersion("version"), "namespace", "", api.Resource("pods").WithVersion("version"), tt.subresource, tt.operation, &metav1.CreateOptions{}, false, &user.DefaultInfo{})
err = wh.Validate(context.TODO(), attr, nil)
if tt.wantAllowed {
@@ -847,7 +981,7 @@ func TestDefaultAllow(t *testing.T) {
}
defer server.Close()
wh, err := newImagePolicyWebhook(t, server.URL, clientCert, clientKey, caCert, 0, tt.defaultAllow)
wh, err := newImagePolicyWebhook(server.URL, clientCert, clientKey, caCert, 0, tt.defaultAllow)
if err != nil {
t.Errorf("%s: failed to create client: %v", tt.test, err)
return
@@ -954,7 +1088,7 @@ func TestAnnotationFiltering(t *testing.T) {
}
defer server.Close()
wh, err := newImagePolicyWebhook(t, server.URL, clientCert, clientKey, caCert, 0, true)
wh, err := newImagePolicyWebhook(server.URL, clientCert, clientKey, caCert, 0, true)
if err != nil {
t.Errorf("%s: failed to create client: %v", tt.test, err)
return
@@ -1047,7 +1181,7 @@ func TestReturnedAnnotationAdd(t *testing.T) {
}
defer server.Close()
wh, err := newImagePolicyWebhook(t, server.URL, clientCert, clientKey, caCert, 0, true)
wh, err := newImagePolicyWebhook(server.URL, clientCert, clientKey, caCert, 0, true)
if err != nil {
t.Errorf("%s: failed to create client: %v", tt.test, err)
return

57
plugin/pkg/admission/serviceaccount/admission.go
View File

@@ -99,7 +99,7 @@ var _ = genericadmissioninitializer.WantsExternalKubeInformerFactory(&Plugin{})
// 5. If MountServiceAccountToken is true, it adds a VolumeMount with the pod's ServiceAccount's api token secret to containers
func NewServiceAccount() *Plugin {
return &Plugin{
Handler: admission.NewHandler(admission.Create),
Handler: admission.NewHandler(admission.Create, admission.Update),
// TODO: enable this once we've swept secret usage to account for adding secret references to service accounts
LimitSecretReferences: false,
// Auto mount service account API token secrets
@@ -139,7 +139,10 @@ func (s *Plugin) Admit(ctx context.Context, a admission.Attributes, o admission.
if shouldIgnore(a) {
return nil
}
if a.GetOperation() != admission.Create {
// we only mutate pods during create requests
return nil
}
pod := a.GetObject().(*api.Pod)
// Don't modify the spec of mirror pods.
@@ -156,7 +159,7 @@ func (s *Plugin) Admit(ctx context.Context, a admission.Attributes, o admission.
serviceAccount, err := s.getServiceAccount(a.GetNamespace(), pod.Spec.ServiceAccountName)
if err != nil {
return admission.NewForbidden(a, fmt.Errorf("error looking up service account %s/%s: %v", a.GetNamespace(), pod.Spec.ServiceAccountName, err))
return admission.NewForbidden(a, fmt.Errorf("error looking up service account %s/%s: %w", a.GetNamespace(), pod.Spec.ServiceAccountName, err))
}
if s.MountServiceAccountToken && shouldAutomount(serviceAccount, pod) {
s.mountServiceAccountToken(serviceAccount, pod)
@@ -179,6 +182,15 @@ func (s *Plugin) Validate(ctx context.Context, a admission.Attributes, o admissi
pod := a.GetObject().(*api.Pod)
if a.GetOperation() == admission.Update && a.GetSubresource() == "ephemeralcontainers" {
return s.limitEphemeralContainerSecretReferences(pod, a)
}
if a.GetOperation() != admission.Create {
// we only validate pod specs during create requests
return nil
}
// Mirror pods have restrictions on what they can reference
if _, isMirrorPod := pod.Annotations[api.MirrorPodAnnotationKey]; isMirrorPod {
if len(pod.Spec.ServiceAccountName) != 0 {
@@ -204,6 +216,10 @@ func (s *Plugin) Validate(ctx context.Context, a admission.Attributes, o admissi
return nil
}
// Require container pods to have service accounts
if len(pod.Spec.ServiceAccountName) == 0 {
return admission.NewForbidden(a, fmt.Errorf("no service account specified for pod %s/%s", a.GetNamespace(), pod.Name))
}
// Ensure the referenced service account exists
serviceAccount, err := s.getServiceAccount(a.GetNamespace(), pod.Spec.ServiceAccountName)
if err != nil {
@@ -220,10 +236,7 @@ func (s *Plugin) Validate(ctx context.Context, a admission.Attributes, o admissi
}
func shouldIgnore(a admission.Attributes) bool {
if a.GetResource().GroupResource() != api.Resource("pods") {
return true
}
if a.GetSubresource() != "" {
if a.GetResource().GroupResource() != api.Resource("pods") || (a.GetSubresource() != "" && a.GetSubresource() != "ephemeralcontainers") {
return true
}
obj := a.GetObject()
@@ -349,6 +362,36 @@ func (s *Plugin) limitSecretReferences(serviceAccount *corev1.ServiceAccount, po
return nil
}
func (s *Plugin) limitEphemeralContainerSecretReferences(pod *api.Pod, a admission.Attributes) error {
// Require ephemeral container pods to have service accounts
if len(pod.Spec.ServiceAccountName) == 0 {
return admission.NewForbidden(a, fmt.Errorf("no service account specified for pod %s/%s", a.GetNamespace(), pod.Name))
}
// Ensure the referenced service account exists
serviceAccount, err := s.getServiceAccount(a.GetNamespace(), pod.Spec.ServiceAccountName)
if err != nil {
return admission.NewForbidden(a, fmt.Errorf("error looking up service account %s/%s: %w", a.GetNamespace(), pod.Spec.ServiceAccountName, err))
}
if !s.enforceMountableSecrets(serviceAccount) {
return nil
}
// Ensure all secrets the ephemeral containers reference are allowed by the service account
mountableSecrets := sets.NewString()
for _, s := range serviceAccount.Secrets {
mountableSecrets.Insert(s.Name)
}
for _, container := range pod.Spec.EphemeralContainers {
for _, env := range container.Env {
if env.ValueFrom != nil && env.ValueFrom.SecretKeyRef != nil {
if !mountableSecrets.Has(env.ValueFrom.SecretKeyRef.Name) {
return fmt.Errorf("ephemeral container %s with envVar %s referencing secret.secretName=\"%s\" is not allowed because service account %s does not reference that secret", container.Name, env.Name, env.ValueFrom.SecretKeyRef.Name, serviceAccount.Name)
}
}
}
}
return nil
}
func (s *Plugin) mountServiceAccountToken(serviceAccount *corev1.ServiceAccount, pod *api.Pod) {
// Find the volume and volume name for the ServiceAccountTokenSecret if it already exists
tokenVolumeName := ""

88
plugin/pkg/admission/serviceaccount/admission_test.go
View File

@@ -543,6 +543,34 @@ func TestAllowsReferencedSecret(t *testing.T) {
if err := admissiontesting.WithReinvocationTesting(t, admit).Admit(context.TODO(), attrs, nil); err != nil {
t.Errorf("Unexpected error: %v", err)
}
pod2 = &api.Pod{
Spec: api.PodSpec{
ServiceAccountName: DefaultServiceAccountName,
EphemeralContainers: []api.EphemeralContainer{
{
EphemeralContainerCommon: api.EphemeralContainerCommon{
Name: "container-2",
Env: []api.EnvVar{
{
Name: "env-1",
ValueFrom: &api.EnvVarSource{
SecretKeyRef: &api.SecretKeySelector{
LocalObjectReference: api.LocalObjectReference{Name: "foo"},
},
},
},
},
},
},
},
},
}
// validate enforces restrictions on secret mounts when operation==create and subresource=='' or operation==update and subresource==ephemeralcontainers"
attrs = admission.NewAttributesRecord(pod2, nil, api.Kind("Pod").WithVersion("version"), ns, "myname", api.Resource("pods").WithVersion("version"), "ephemeralcontainers", admission.Update, &metav1.UpdateOptions{}, false, nil)
if err := admit.Validate(context.TODO(), attrs, nil); err != nil {
t.Errorf("Unexpected error: %v", err)
}
}
func TestRejectsUnreferencedSecretVolumes(t *testing.T) {
@@ -620,6 +648,66 @@ func TestRejectsUnreferencedSecretVolumes(t *testing.T) {
if err := admissiontesting.WithReinvocationTesting(t, admit).Admit(context.TODO(), attrs, nil); err == nil || !strings.Contains(err.Error(), "with envVar") {
t.Errorf("Unexpected error: %v", err)
}
pod2 = &api.Pod{
Spec: api.PodSpec{
ServiceAccountName: DefaultServiceAccountName,
InitContainers: []api.Container{
{
Name: "container-1",
Env: []api.EnvVar{
{
Name: "env-1",
ValueFrom: &api.EnvVarSource{
SecretKeyRef: &api.SecretKeySelector{
LocalObjectReference: api.LocalObjectReference{Name: "foo"},
},
},
},
},
},
},
},
}
attrs = admission.NewAttributesRecord(pod2, nil, api.Kind("Pod").WithVersion("version"), ns, "myname", api.Resource("pods").WithVersion("version"), "", admission.Update, &metav1.UpdateOptions{}, false, nil)
if err := admissiontesting.WithReinvocationTesting(t, admit).Admit(context.TODO(), attrs, nil); err != nil {
t.Errorf("admit only enforces restrictions on secret mounts when operation==create. Unexpected error: %v", err)
}
attrs = admission.NewAttributesRecord(pod2, nil, api.Kind("Pod").WithVersion("version"), ns, "myname", api.Resource("pods").WithVersion("version"), "", admission.Create, &metav1.CreateOptions{}, false, nil)
if err := admit.Validate(context.TODO(), attrs, nil); err == nil || !strings.Contains(err.Error(), "with envVar") {
t.Errorf("validate only enforces restrictions on secret mounts when operation==create and subresource==''. Unexpected error: %v", err)
}
pod2 = &api.Pod{
Spec: api.PodSpec{
ServiceAccountName: DefaultServiceAccountName,
EphemeralContainers: []api.EphemeralContainer{
{
EphemeralContainerCommon: api.EphemeralContainerCommon{
Name: "container-2",
Env: []api.EnvVar{
{
Name: "env-1",
ValueFrom: &api.EnvVarSource{
SecretKeyRef: &api.SecretKeySelector{
LocalObjectReference: api.LocalObjectReference{Name: "foo"},
},
},
},
},
},
},
},
},
}
attrs = admission.NewAttributesRecord(pod2, nil, api.Kind("Pod").WithVersion("version"), ns, "myname", api.Resource("pods").WithVersion("version"), "", admission.Update, &metav1.UpdateOptions{}, false, nil)
if err := admissiontesting.WithReinvocationTesting(t, admit).Admit(context.TODO(), attrs, nil); err != nil {
t.Errorf("admit only enforces restrictions on secret mounts when operation==create and subresource==''. Unexpected error: %v", err)
}
attrs = admission.NewAttributesRecord(pod2, nil, api.Kind("Pod").WithVersion("version"), ns, "myname", api.Resource("pods").WithVersion("version"), "ephemeralcontainers", admission.Update, &metav1.UpdateOptions{}, false, nil)
if err := admit.Validate(context.TODO(), attrs, nil); err == nil || !strings.Contains(err.Error(), "with envVar") {
t.Errorf("validate enforces restrictions on secret mounts when operation==update and subresource==ephemeralcontainers. Unexpected error: %v", err)
}
}
func TestAllowUnreferencedSecretVolumesForPermissiveSAs(t *testing.T) {