Skip to content
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

Automated cherry pick of #118356: Add ephemeralcontainer to imagepolicy securityaccount #118473

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
28 changes: 19 additions & 9 deletions plugin/pkg/admission/imagepolicy/admission.go
Expand Up @@ -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.
Expand Down Expand Up @@ -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
}

Expand All @@ -144,13 +146,21 @@ func (a *Plugin) Validate(ctx context.Context, attributes admission.Attributes,

// Build list of ImageReviewContainerSpec
var imageReviewContainerSpecs []v1alpha1.ImageReviewContainerSpec
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...)
for _, c := range containers {
imageReviewContainerSpecs = append(imageReviewContainerSpecs, v1alpha1.ImageReviewContainerSpec{
Image: c.Image,
})
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...)
for _, c := range containers {
imageReviewContainerSpecs = append(imageReviewContainerSpecs, v1alpha1.ImageReviewContainerSpec{
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{
Expand Down
148 changes: 140 additions & 8 deletions plugin/pkg/admission/imagepolicy/admission_test.go
Expand Up @@ -37,7 +37,6 @@ import (
api "k8s.io/kubernetes/pkg/apis/core"

"fmt"
"io/ioutil"
"os"
"path/filepath"
"text/template"
Expand Down Expand Up @@ -67,7 +66,7 @@ imagePolicy:
`

func TestNewFromConfig(t *testing.T) {
dir, err := ioutil.TempDir("", "")
dir, err := os.MkdirTemp("", "")
if err != nil {
t.Fatal(err)
}
Expand All @@ -92,7 +91,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)
}
}
Expand Down Expand Up @@ -196,7 +195,7 @@ 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
}
Expand All @@ -211,7 +210,7 @@ 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
}
Expand Down Expand Up @@ -359,7 +358,7 @@ 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(callbackURL string, clientCert, clientKey, ca []byte, cacheTime time.Duration, defaultAllow bool) (*Plugin, error) {
tempfile, err := ioutil.TempFile("", "")
tempfile, err := os.CreateTemp("", "")
if err != nil {
return nil, err
}
Expand All @@ -381,7 +380,7 @@ func newImagePolicyWebhook(callbackURL string, clientCert, clientKey, ca []byte,
return nil, err
}

tempconfigfile, err := ioutil.TempFile("", "")
tempconfigfile, err := os.CreateTemp("", "")
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -595,17 +594,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",
Expand All @@ -627,6 +632,8 @@ func TestContainerCombinations(t *testing.T) {
},
wantAllowed: false,
wantErr: true,
subresource: "",
operation: admission.Create,
},
{
test: "Multiple good containers",
Expand All @@ -648,6 +655,8 @@ func TestContainerCombinations(t *testing.T) {
},
wantAllowed: true,
wantErr: false,
subresource: "",
operation: admission.Create,
},
{
test: "Multiple bad containers",
Expand All @@ -669,6 +678,8 @@ func TestContainerCombinations(t *testing.T) {
},
wantAllowed: false,
wantErr: true,
subresource: "",
operation: admission.Create,
},
{
test: "Good container, bad init container",
Expand All @@ -692,6 +703,8 @@ func TestContainerCombinations(t *testing.T) {
},
wantAllowed: false,
wantErr: true,
subresource: "",
operation: admission.Create,
},
{
test: "Bad container, good init container",
Expand All @@ -715,6 +728,8 @@ func TestContainerCombinations(t *testing.T) {
},
wantAllowed: false,
wantErr: true,
subresource: "",
operation: admission.Create,
},
{
test: "Good container, good init container",
Expand All @@ -738,6 +753,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 {
Expand All @@ -759,7 +891,7 @@ func TestContainerCombinations(t *testing.T) {
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 {
Expand Down