From 0e69ed4bec56d7956457802225a45b962e7869be Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 31 May 2024 15:05:32 +0800 Subject: [PATCH 1/6] Bump actions/checkout from 2 to 4 (#1590) Bumps [actions/checkout](https://github.com/actions/checkout) from 2 to 4. - [Release notes](https://github.com/actions/checkout/releases) - [Commits](https://github.com/actions/checkout/compare/v2...v4) --- updated-dependencies: - dependency-name: actions/checkout dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/ci.yaml | 8 ++++---- .github/workflows/codeql.yml | 2 +- .github/workflows/docker-image.yaml | 2 +- .github/workflows/e2e-1.18.yaml | 12 ++++++------ .github/workflows/e2e-1.20-EphemeralJob.yaml | 2 +- .github/workflows/e2e-1.24.yaml | 14 +++++++------- .github/workflows/e2e-1.26.yaml | 14 +++++++------- .github/workflows/e2e-1.28.yaml | 14 +++++++------- .github/workflows/license.yml | 2 +- .github/workflows/scorecard.yml | 2 +- 10 files changed, 36 insertions(+), 36 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 530cfc2435..caa2e4e748 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -29,7 +29,7 @@ jobs: runs-on: ubuntu-20.04 steps: - name: Checkout Actions Repository - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Check spelling with custom config file uses: crate-ci/typos@v1.21.0 with: @@ -41,7 +41,7 @@ jobs: security-events: write steps: - name: Checkout Code - uses: actions/checkout@v3 + uses: actions/checkout@v4 with: submodules: true - name: Setup Go @@ -84,7 +84,7 @@ jobs: container: pouchcontainer/pouchlinter:v0.1.2 steps: - name: Checkout - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Run misspell run: find ./* -name "*" | grep -v vendor | xargs misspell -error - name: Run shellcheck @@ -107,7 +107,7 @@ jobs: unit-tests: runs-on: ubuntu-20.04 steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 with: submodules: true - name: Fetch History diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index 67b5f74dd4..25dda2b34e 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -45,7 +45,7 @@ jobs: steps: - name: Checkout repository - uses: actions/checkout@v3 + uses: actions/checkout@v4 # Initializes the CodeQL tools for scanning. - name: Initialize CodeQL diff --git a/.github/workflows/docker-image.yaml b/.github/workflows/docker-image.yaml index 09ac3738dc..b400f2be3f 100644 --- a/.github/workflows/docker-image.yaml +++ b/.github/workflows/docker-image.yaml @@ -13,6 +13,6 @@ jobs: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - name: Build the Docker image run: IMG=openkruise/kruise-manager:${{ github.ref_name }} & make docker-multiarch diff --git a/.github/workflows/e2e-1.18.yaml b/.github/workflows/e2e-1.18.yaml index 07c6327ca8..8fff10a3d7 100644 --- a/.github/workflows/e2e-1.18.yaml +++ b/.github/workflows/e2e-1.18.yaml @@ -23,7 +23,7 @@ jobs: astatefulset: runs-on: ubuntu-20.04 steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 with: submodules: true - name: Setup Go @@ -115,7 +115,7 @@ jobs: pullimages-containerrecreate: runs-on: ubuntu-20.04 steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 with: submodules: true - name: Setup Go @@ -206,7 +206,7 @@ jobs: advanced-daemonset: runs-on: ubuntu-20.04 steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 with: submodules: true - name: Setup Go @@ -297,7 +297,7 @@ jobs: sidecarset: runs-on: ubuntu-20.04 steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 with: submodules: true - name: Setup Go @@ -388,7 +388,7 @@ jobs: podUnavailableBudget: runs-on: ubuntu-20.04 steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 with: submodules: true - name: Setup Go @@ -453,7 +453,7 @@ jobs: other: runs-on: ubuntu-20.04 steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 with: submodules: true - name: Setup Go diff --git a/.github/workflows/e2e-1.20-EphemeralJob.yaml b/.github/workflows/e2e-1.20-EphemeralJob.yaml index db323b0954..bb1979ac1b 100644 --- a/.github/workflows/e2e-1.20-EphemeralJob.yaml +++ b/.github/workflows/e2e-1.20-EphemeralJob.yaml @@ -22,7 +22,7 @@ jobs: ephemeraljob: runs-on: ubuntu-20.04 steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 with: submodules: true - name: Setup Go diff --git a/.github/workflows/e2e-1.24.yaml b/.github/workflows/e2e-1.24.yaml index 2c53b4bb6d..4177cf2cd8 100644 --- a/.github/workflows/e2e-1.24.yaml +++ b/.github/workflows/e2e-1.24.yaml @@ -24,7 +24,7 @@ jobs: astatefulset: runs-on: ubuntu-20.04 steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 with: submodules: true - name: Setup Go @@ -103,7 +103,7 @@ jobs: pullimages-containerrecreate: runs-on: ubuntu-20.04 steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 with: submodules: true - name: Setup Go @@ -195,7 +195,7 @@ jobs: advanced-daemonset: runs-on: ubuntu-20.04 steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 with: submodules: true - name: Setup Go @@ -287,7 +287,7 @@ jobs: sidecarset: runs-on: ubuntu-20.04 steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 with: submodules: true - name: Setup Go @@ -379,7 +379,7 @@ jobs: ephemeraljob: runs-on: ubuntu-20.04 steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 with: submodules: true - name: Setup Go @@ -449,7 +449,7 @@ jobs: podUnavailableBudget: runs-on: ubuntu-20.04 steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 with: submodules: true - name: Setup Go @@ -514,7 +514,7 @@ jobs: other: runs-on: ubuntu-20.04 steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 with: submodules: true - name: Setup Go diff --git a/.github/workflows/e2e-1.26.yaml b/.github/workflows/e2e-1.26.yaml index c8b55b18fa..16cdc81ee3 100644 --- a/.github/workflows/e2e-1.26.yaml +++ b/.github/workflows/e2e-1.26.yaml @@ -23,7 +23,7 @@ jobs: astatefulset: runs-on: ubuntu-20.04 steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 with: submodules: true - name: Setup Go @@ -102,7 +102,7 @@ jobs: pullimages-containerrecreate: runs-on: ubuntu-20.04 steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 with: submodules: true - name: Setup Go @@ -194,7 +194,7 @@ jobs: advanced-daemonset: runs-on: ubuntu-20.04 steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 with: submodules: true - name: Setup Go @@ -286,7 +286,7 @@ jobs: sidecarset: runs-on: ubuntu-20.04 steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 with: submodules: true - name: Setup Go @@ -378,7 +378,7 @@ jobs: ephemeraljob: runs-on: ubuntu-20.04 steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 with: submodules: true - name: Setup Go @@ -448,7 +448,7 @@ jobs: podUnavailableBudget: runs-on: ubuntu-20.04 steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 with: submodules: true - name: Setup Go @@ -513,7 +513,7 @@ jobs: other: runs-on: ubuntu-20.04 steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 with: submodules: true - name: Setup Go diff --git a/.github/workflows/e2e-1.28.yaml b/.github/workflows/e2e-1.28.yaml index 5d5c6e30e1..8218a5fe28 100644 --- a/.github/workflows/e2e-1.28.yaml +++ b/.github/workflows/e2e-1.28.yaml @@ -23,7 +23,7 @@ jobs: astatefulset: runs-on: ubuntu-20.04 steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 with: submodules: true - name: Setup Go @@ -102,7 +102,7 @@ jobs: pullimages-containerrecreate: runs-on: ubuntu-20.04 steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 with: submodules: true - name: Setup Go @@ -194,7 +194,7 @@ jobs: advanced-daemonset: runs-on: ubuntu-20.04 steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 with: submodules: true - name: Setup Go @@ -286,7 +286,7 @@ jobs: sidecarset: runs-on: ubuntu-20.04 steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 with: submodules: true - name: Setup Go @@ -378,7 +378,7 @@ jobs: ephemeraljob: runs-on: ubuntu-20.04 steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 with: submodules: true - name: Setup Go @@ -448,7 +448,7 @@ jobs: podUnavailableBudget: runs-on: ubuntu-20.04 steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 with: submodules: true - name: Setup Go @@ -513,7 +513,7 @@ jobs: other: runs-on: ubuntu-20.04 steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 with: submodules: true - name: Setup Go diff --git a/.github/workflows/license.yml b/.github/workflows/license.yml index 9d1bb0a73d..3242834c66 100644 --- a/.github/workflows/license.yml +++ b/.github/workflows/license.yml @@ -18,7 +18,7 @@ jobs: runs-on: ubuntu-20.04 name: Check for unapproved licenses steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v4 - name: Set up Ruby uses: ruby/setup-ruby@v1 with: diff --git a/.github/workflows/scorecard.yml b/.github/workflows/scorecard.yml index 190fbf3dd6..cbb88f3a32 100644 --- a/.github/workflows/scorecard.yml +++ b/.github/workflows/scorecard.yml @@ -32,7 +32,7 @@ jobs: steps: - name: "Checkout code" - uses: actions/checkout@93ea575cb5d8a053eaa0ac8fa3b40d7e05a33cc8 # v3.1.0 + uses: actions/checkout@44c2b7a8a4ea60a981eaca3cf939b5f4305c123b # v4.1.5 with: persist-credentials: false From 6d3199bb7429aee8b0540f31fb36f006a6fea5af Mon Sep 17 00:00:00 2001 From: berg Date: Tue, 4 Jun 2024 09:22:36 +0800 Subject: [PATCH 2/6] sidecarSet initContainer support InjectPolicy (#1617) Signed-off-by: liheng.zms --- apis/apps/defaults/v1alpha1.go | 12 +- pkg/webhook/pod/mutating/sidecarset.go | 7 +- pkg/webhook/pod/mutating/sidecarset_test.go | 143 +++++++++++++++++- .../sidecarset_create_update_handler.go | 3 + test/e2e/apps/containerrecreate.go | 5 +- test/e2e/apps/sidecarset.go | 2 + test/e2e/framework/containerrecreate_util.go | 3 + 7 files changed, 161 insertions(+), 14 deletions(-) diff --git a/apis/apps/defaults/v1alpha1.go b/apis/apps/defaults/v1alpha1.go index 9a15eaeb80..89baa72574 100644 --- a/apis/apps/defaults/v1alpha1.go +++ b/apis/apps/defaults/v1alpha1.go @@ -37,11 +37,11 @@ func SetDefaultsSidecarSet(obj *v1alpha1.SidecarSet) { setSidecarSetUpdateStrategy(&obj.Spec.UpdateStrategy) for i := range obj.Spec.InitContainers { - setSidecarDefaultContainer(&obj.Spec.InitContainers[i]) + setDefaultSidecarContainer(&obj.Spec.InitContainers[i], v1alpha1.AfterAppContainerType) } for i := range obj.Spec.Containers { - setDefaultSidecarContainer(&obj.Spec.Containers[i]) + setDefaultSidecarContainer(&obj.Spec.Containers[i], v1alpha1.BeforeAppContainerType) } //default setting volumes @@ -74,9 +74,9 @@ func SetDefaultRevisionHistoryLimit(revisionHistoryLimit **int32) { } } -func setDefaultSidecarContainer(sidecarContainer *v1alpha1.SidecarContainer) { +func setDefaultSidecarContainer(sidecarContainer *v1alpha1.SidecarContainer, injectPolicy v1alpha1.PodInjectPolicyType) { if sidecarContainer.PodInjectPolicy == "" { - sidecarContainer.PodInjectPolicy = v1alpha1.BeforeAppContainerType + sidecarContainer.PodInjectPolicy = injectPolicy } if sidecarContainer.UpgradeStrategy.UpgradeType == "" { sidecarContainer.UpgradeStrategy.UpgradeType = v1alpha1.SidecarContainerColdUpgrade @@ -85,7 +85,7 @@ func setDefaultSidecarContainer(sidecarContainer *v1alpha1.SidecarContainer) { sidecarContainer.ShareVolumePolicy.Type = v1alpha1.ShareVolumePolicyDisabled } - setSidecarDefaultContainer(sidecarContainer) + setDefaultContainer(sidecarContainer) } func setSidecarSetUpdateStrategy(strategy *v1alpha1.SidecarSetUpdateStrategy) { @@ -102,7 +102,7 @@ func setSidecarSetUpdateStrategy(strategy *v1alpha1.SidecarSetUpdateStrategy) { } } -func setSidecarDefaultContainer(sidecarContainer *v1alpha1.SidecarContainer) { +func setDefaultContainer(sidecarContainer *v1alpha1.SidecarContainer) { container := &sidecarContainer.Container v1.SetDefaults_Container(container) for i := range container.Ports { diff --git a/pkg/webhook/pod/mutating/sidecarset.go b/pkg/webhook/pod/mutating/sidecarset.go index 194738c095..21528d0778 100644 --- a/pkg/webhook/pod/mutating/sidecarset.go +++ b/pkg/webhook/pod/mutating/sidecarset.go @@ -152,10 +152,7 @@ func (h *PodCreateHandler) sidecarsetMutatingPod(ctx context.Context, req admiss sort.SliceStable(sidecarInitContainers, func(i, j int) bool { return sidecarInitContainers[i].Name < sidecarInitContainers[j].Name }) - // TODO, implement PodInjectPolicy for initContainers - for _, initContainer := range sidecarInitContainers { - pod.Spec.InitContainers = append(pod.Spec.InitContainers, initContainer.Container) - } + pod.Spec.InitContainers = mergeSidecarContainers(pod.Spec.InitContainers, sidecarInitContainers) // 2. inject containers pod.Spec.Containers = mergeSidecarContainers(pod.Spec.Containers, sidecarContainers) // 3. inject volumes @@ -287,7 +284,7 @@ func mergeSidecarContainers(origins []corev1.Container, injected []*appsv1alpha1 case appsv1alpha1.AfterAppContainerType: afterAppContainers = append(afterAppContainers, sidecar.Container) default: - beforeAppContainers = append(beforeAppContainers, sidecar.Container) + afterAppContainers = append(afterAppContainers, sidecar.Container) } } origins = append(beforeAppContainers, origins...) diff --git a/pkg/webhook/pod/mutating/sidecarset_test.go b/pkg/webhook/pod/mutating/sidecarset_test.go index 517c661180..f3f6f83062 100644 --- a/pkg/webhook/pod/mutating/sidecarset_test.go +++ b/pkg/webhook/pod/mutating/sidecarset_test.go @@ -23,6 +23,7 @@ import ( "os" "path/filepath" "reflect" + "sort" "testing" "github.com/openkruise/kruise/apis" @@ -946,6 +947,7 @@ func TestSidecarSetTransferEnv(t *testing.T) { func testSidecarSetTransferEnv(t *testing.T, sidecarSetIn *appsv1alpha1.SidecarSet) { podIn := pod1.DeepCopy() + sidecarSetIn.Spec.InitContainers[0].PodInjectPolicy = "" decoder := admission.NewDecoder(scheme.Scheme) client := fake.NewClientBuilder().WithObjects(sidecarSetIn).WithIndex( &appsv1alpha1.SidecarSet{}, fieldindex.IndexNameForSidecarSetNamespace, fieldindex.IndexSidecarSet, @@ -958,7 +960,7 @@ func testSidecarSetTransferEnv(t *testing.T, sidecarSetIn *appsv1alpha1.SidecarS t.Fatalf("inject sidecar into pod failed, err: %v", err) } if len(podOut.Spec.InitContainers[1].Env) != 2 { - t.Fatalf("expect 2 envs but got %v", len(podOut.Spec.InitContainers[0].Env)) + t.Fatalf("expect 2 envs but got %v", len(podOut.Spec.InitContainers[1].Env)) } if podOut.Spec.InitContainers[1].Env[1].Value != "world2" { t.Fatalf("expect env with value 'world2' but got %v", podOut.Spec.Containers[0].Env[1].Value) @@ -1129,6 +1131,17 @@ func TestMergeSidecarContainers(t *testing.T) { expectContainerLen: 4, expectedContainers: []string{"new-sidecar-1", "sidecar-1", "app-container", "sidecar-2"}, }, + { + name: "origin no sidecar, sidecar have new containers", + getOrigins: func() []corev1.Container { + return nil + }, + getInjected: func() []*appsv1alpha1.SidecarContainer { + return sidecarContainers + }, + expectContainerLen: 3, + expectedContainers: []string{"new-sidecar-1", "sidecar-1", "sidecar-2"}, + }, } for _, cs := range cases { @@ -1255,6 +1268,134 @@ func TestInjectInitContainer(t *testing.T) { } } +func TestInjectInitContainerSort(t *testing.T) { + cases := []struct { + name string + getOrigins func() []corev1.Container + getInjected func() []*appsv1alpha1.SidecarContainer + expectContainerLen int + expectedContainers []string + }{ + { + name: "origins nil, inject containers(a, b, c)", + getOrigins: func() []corev1.Container { + return nil + }, + getInjected: func() []*appsv1alpha1.SidecarContainer { + return []*appsv1alpha1.SidecarContainer{ + { + Container: corev1.Container{ + Name: "a-init", + }, + }, + { + Container: corev1.Container{ + Name: "c-init", + }, + }, + { + Container: corev1.Container{ + Name: "b-init", + }, + }, + } + }, + expectContainerLen: 3, + expectedContainers: []string{"a-init", "b-init", "c-init"}, + }, + { + name: "origin init, inject containers(a, b, c)", + getOrigins: func() []corev1.Container { + return []corev1.Container{ + { + Name: "app1-init", + }, + { + Name: "app2-init", + }, + } + }, + getInjected: func() []*appsv1alpha1.SidecarContainer { + return []*appsv1alpha1.SidecarContainer{ + { + Container: corev1.Container{ + Name: "a-init", + }, + }, + { + Container: corev1.Container{ + Name: "c-init", + }, + }, + { + Container: corev1.Container{ + Name: "b-init", + }, + }, + } + }, + expectContainerLen: 5, + expectedContainers: []string{"app1-init", "app2-init", "a-init", "b-init", "c-init"}, + }, + { + name: "origins nil, inject containers(a, b, c)-2", + getOrigins: func() []corev1.Container { + return []corev1.Container{ + { + Name: "app1-init", + }, + { + Name: "app2-init", + }, + } + }, + getInjected: func() []*appsv1alpha1.SidecarContainer { + return []*appsv1alpha1.SidecarContainer{ + { + Container: corev1.Container{ + Name: "a-init", + }, + PodInjectPolicy: appsv1alpha1.AfterAppContainerType, + }, + { + Container: corev1.Container{ + Name: "c-init", + }, + PodInjectPolicy: appsv1alpha1.BeforeAppContainerType, + }, + { + Container: corev1.Container{ + Name: "b-init", + }, + PodInjectPolicy: appsv1alpha1.BeforeAppContainerType, + }, + } + }, + expectContainerLen: 5, + expectedContainers: []string{"b-init", "c-init", "app1-init", "app2-init", "a-init"}, + }, + } + + for _, cs := range cases { + t.Run(cs.name, func(t *testing.T) { + origins := cs.getOrigins() + injected := cs.getInjected() + sort.SliceStable(injected, func(i, j int) bool { + return injected[i].Name < injected[j].Name + }) + finals := mergeSidecarContainers(origins, injected) + if len(finals) != cs.expectContainerLen { + t.Fatalf("expect %d containers but got %v", cs.expectContainerLen, len(finals)) + } + for index, cName := range cs.expectedContainers { + if finals[index].Name != cName { + t.Fatalf("expect index(%d) container(%s) but got %s", index, cName, finals[index].Name) + } + } + }) + } +} + func newAdmission(op admissionv1.Operation, object, oldObject runtime.RawExtension, subResource string) admission.Request { return admission.Request{ AdmissionRequest: newAdmissionRequest(op, object, oldObject, subResource), diff --git a/pkg/webhook/sidecarset/validating/sidecarset_create_update_handler.go b/pkg/webhook/sidecarset/validating/sidecarset_create_update_handler.go index 8dbf4dc5b9..4c1584c0f1 100644 --- a/pkg/webhook/sidecarset/validating/sidecarset_create_update_handler.go +++ b/pkg/webhook/sidecarset/validating/sidecarset_create_update_handler.go @@ -90,6 +90,9 @@ func (h *SidecarSetCreateUpdateHandler) validateSidecarSet(obj *appsv1alpha1.Sid if older != nil { allErrs = append(allErrs, validateSidecarContainerConflict(obj.Spec.Containers, older.Spec.Containers, field.NewPath("spec.containers"))...) } + if len(allErrs) != 0 { + return allErrs + } // iterate across all containers in other sidecarsets to avoid duplication of name sidecarSets := &appsv1alpha1.SidecarSetList{} if err := h.Client.List(context.TODO(), sidecarSets, &client.ListOptions{}); err != nil { diff --git a/test/e2e/apps/containerrecreate.go b/test/e2e/apps/containerrecreate.go index bac02d2190..113ee6d85e 100644 --- a/test/e2e/apps/containerrecreate.go +++ b/test/e2e/apps/containerrecreate.go @@ -20,12 +20,11 @@ import ( "fmt" "time" - "github.com/openkruise/kruise/pkg/util" - "github.com/onsi/ginkgo" "github.com/onsi/gomega" appsv1alpha1 "github.com/openkruise/kruise/apis/apps/v1alpha1" kruiseclientset "github.com/openkruise/kruise/pkg/client/clientset/versioned" + "github.com/openkruise/kruise/pkg/util" utilpodreadiness "github.com/openkruise/kruise/pkg/util/podreadiness" "github.com/openkruise/kruise/test/e2e/framework" v1 "k8s.io/api/core/v1" @@ -33,6 +32,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/rand" clientset "k8s.io/client-go/kubernetes" + "k8s.io/klog/v2" podutil "k8s.io/kubernetes/pkg/api/v1/pod" utilpointer "k8s.io/utils/pointer" ) @@ -216,6 +216,7 @@ var _ = SIGDescribe("ContainerRecreateRequest", func() { gomega.Expect(err).NotTo(gomega.HaveOccurred()) return crr.Status.Phase }, 60*time.Second, 3*time.Second).Should(gomega.Equal(appsv1alpha1.ContainerRecreateRequestCompleted)) + klog.Infof("CRR info(%s)", util.DumpJSON(crr)) gomega.Expect(crr.Status.CompletionTime).ShouldNot(gomega.BeNil()) gomega.Expect(crr.Status.ContainerRecreateStates).Should(gomega.Equal([]appsv1alpha1.ContainerRecreateRequestContainerRecreateState{ {Name: "app", Phase: appsv1alpha1.ContainerRecreateRequestSucceeded, IsKilled: true}, diff --git a/test/e2e/apps/sidecarset.go b/test/e2e/apps/sidecarset.go index 1667cea830..6acfd4a6f4 100644 --- a/test/e2e/apps/sidecarset.go +++ b/test/e2e/apps/sidecarset.go @@ -1328,6 +1328,8 @@ var _ = SIGDescribe("SidecarSet", func() { sidecarSetIn.Spec.Containers = sidecarSetIn.Spec.Containers[:1] ginkgo.By(fmt.Sprintf("Creating SidecarSet %s", sidecarSetIn.Name)) sidecarSetIn, _ = tester.CreateSidecarSet(sidecarSetIn) + gomega.Expect(sidecarSetIn.Spec.InitContainers[0].PodInjectPolicy).To(gomega.Equal(appsv1alpha1.AfterAppContainerType)) + gomega.Expect(sidecarSetIn.Spec.Containers[0].PodInjectPolicy).To(gomega.Equal(appsv1alpha1.BeforeAppContainerType)) time.Sleep(time.Second) // create deployment diff --git a/test/e2e/framework/containerrecreate_util.go b/test/e2e/framework/containerrecreate_util.go index ce48465aa0..8e5fd82d07 100644 --- a/test/e2e/framework/containerrecreate_util.go +++ b/test/e2e/framework/containerrecreate_util.go @@ -23,9 +23,11 @@ import ( "github.com/onsi/gomega" appsv1alpha1 "github.com/openkruise/kruise/apis/apps/v1alpha1" kruiseclientset "github.com/openkruise/kruise/pkg/client/clientset/versioned" + "github.com/openkruise/kruise/pkg/util" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" clientset "k8s.io/client-go/kubernetes" + "k8s.io/klog/v2" ) type ContainerRecreateTester struct { @@ -85,6 +87,7 @@ func (t *ContainerRecreateTester) CreateTestCloneSetAndGetPods(randStr string, r gomega.Expect(err).NotTo(gomega.HaveOccurred()) for i := range podList.Items { p := &podList.Items[i] + klog.Infof("Pod(%s/%s/%s) status(%s)", p.Namespace, p.Name, p.UID, util.DumpJSON(p.Status)) pods = append(pods, p) } return From 1045e6c902bfefe1bc0ca48569e02ef578427c9b Mon Sep 17 00:00:00 2001 From: Abner Date: Tue, 4 Jun 2024 13:45:36 +0800 Subject: [PATCH 3/6] fix markdown linter checkout err (#1638) Signed-off-by: Abner-1 --- .github/workflows/ci.yaml | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index caa2e4e748..f820e93e94 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -77,20 +77,21 @@ jobs: with: sarif_file: 'trivy-results.sarif' - markdownlint-misspell-shellcheck: - runs-on: ubuntu-20.04 - # this image is build from Dockerfile - # https://github.com/pouchcontainer/pouchlinter/blob/master/Dockerfile - container: pouchcontainer/pouchlinter:v0.1.2 - steps: - - name: Checkout - uses: actions/checkout@v4 - - name: Run misspell - run: find ./* -name "*" | grep -v vendor | xargs misspell -error - - name: Run shellcheck - run: find ./ -name "*.sh" | grep -v vendor | xargs shellcheck - - name: Lint markdown files - run: find ./ -name "*.md" | grep -v vendor | grep -v commandline | grep -v .github | grep -v swagger | grep -v api | xargs mdl -r ~MD010,~MD013,~MD014,~MD022,~MD024,~MD029,~MD031,~MD032,~MD033,~MD036 +# markdownlint-misspell-shellcheck: +# runs-on: ubuntu-20.04 +# # this image is build from Dockerfile +# # https://github.com/pouchcontainer/pouchlinter/blob/master/Dockerfile +# container: pouchcontainer/pouchlinter:v0.1.2 +# steps: +# - name: Checkout +# uses: actions/checkout@v3 +# - name: Run misspell +# run: find ./* -name "*" | grep -v vendor | xargs misspell -error +# - name: Run shellcheck +# run: find ./ -name "*.sh" | grep -v vendor | xargs shellcheck +# - name: Lint markdown files +# run: find ./ -name "*.md" | grep -v vendor | grep -v commandline | grep -v .github | grep -v swagger | grep -v api | xargs mdl -r ~MD010,~MD013,~MD014,~MD022,~MD024,~MD029,~MD031,~MD032,~MD033,~MD036 + # - name: Check markdown links # run: | # set +e From eb9a8b6d8109811b9c5a30ba0167f08269a976f7 Mon Sep 17 00:00:00 2001 From: Abner Date: Tue, 4 Jun 2024 16:11:36 +0800 Subject: [PATCH 4/6] add ephemeraljob validating webhook, add validation&ut (#1615) Signed-off-by: Abner-1 --- config/webhook/manifests.yaml | 21 + pkg/webhook/add_ephemeraljob.go | 25 ++ .../ephemeraljob_create_update_handler.go | 70 ++++ ...ephemeraljob_create_update_handler_test.go | 170 ++++++++ .../ephemeraljob/validating/validation.go | 215 ++++++++++ .../validating/validation_test.go | 394 ++++++++++++++++++ .../ephemeraljob/validating/webhooks.go | 35 ++ pkg/webhook/util/convertor/util.go | 15 +- 8 files changed, 944 insertions(+), 1 deletion(-) create mode 100644 pkg/webhook/add_ephemeraljob.go create mode 100644 pkg/webhook/ephemeraljob/validating/ephemeraljob_create_update_handler.go create mode 100644 pkg/webhook/ephemeraljob/validating/ephemeraljob_create_update_handler_test.go create mode 100644 pkg/webhook/ephemeraljob/validating/validation.go create mode 100644 pkg/webhook/ephemeraljob/validating/validation_test.go create mode 100644 pkg/webhook/ephemeraljob/validating/webhooks.go diff --git a/config/webhook/manifests.yaml b/config/webhook/manifests.yaml index 077c069437..7bf8c8baf1 100644 --- a/config/webhook/manifests.yaml +++ b/config/webhook/manifests.yaml @@ -470,6 +470,27 @@ webhooks: resources: - daemonsets sideEffects: None +- admissionReviewVersions: + - v1 + - v1beta1 + clientConfig: + service: + name: webhook-service + namespace: system + path: /validate-apps-kruise-io-v1alpha1-ephemeraljob + failurePolicy: Fail + name: vephemeraljobs.kb.io + rules: + - apiGroups: + - apps.kruise.io + apiVersions: + - v1alpha1 + operations: + - CREATE + - UPDATE + resources: + - ephemeraljobs + sideEffects: None - admissionReviewVersions: - v1 - v1beta1 diff --git a/pkg/webhook/add_ephemeraljob.go b/pkg/webhook/add_ephemeraljob.go new file mode 100644 index 0000000000..ae1348d3c2 --- /dev/null +++ b/pkg/webhook/add_ephemeraljob.go @@ -0,0 +1,25 @@ +/* +Copyright 2024 The Kruise Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package webhook + +import ( + "github.com/openkruise/kruise/pkg/webhook/ephemeraljob/validating" +) + +func init() { + addHandlers(validating.HandlerGetterMap) +} diff --git a/pkg/webhook/ephemeraljob/validating/ephemeraljob_create_update_handler.go b/pkg/webhook/ephemeraljob/validating/ephemeraljob_create_update_handler.go new file mode 100644 index 0000000000..4c71b2f39b --- /dev/null +++ b/pkg/webhook/ephemeraljob/validating/ephemeraljob_create_update_handler.go @@ -0,0 +1,70 @@ +/* +Copyright 2021 The Kruise Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package validating + +import ( + "context" + "net/http" + + "k8s.io/apimachinery/pkg/util/validation/field" + "k8s.io/klog/v2" + "k8s.io/kubernetes/pkg/apis/core/validation" + "sigs.k8s.io/controller-runtime/pkg/manager" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + appsv1alpha1 "github.com/openkruise/kruise/apis/apps/v1alpha1" + "github.com/openkruise/kruise/pkg/webhook/util/convertor" +) + +// EphemeralJobCreateUpdateHandler handles EphemeralJob +type EphemeralJobCreateUpdateHandler struct { + // Decoder decodes objects + Decoder *admission.Decoder +} + +var _ admission.Handler = &EphemeralJobCreateUpdateHandler{} + +func NewHandler(mgr manager.Manager) admission.Handler { + return &EphemeralJobCreateUpdateHandler{Decoder: admission.NewDecoder(mgr.GetScheme())} +} + +// Handle handles admission requests. +func (h *EphemeralJobCreateUpdateHandler) Handle(ctx context.Context, req admission.Request) admission.Response { + obj := &appsv1alpha1.EphemeralJob{} + + err := h.Decoder.Decode(req, obj) + if err != nil { + return admission.Errored(http.StatusBadRequest, err) + } + + if err := validate(obj); err != nil { + klog.Warningf("Error validate EphemeralJob %s: %v", obj.Name, err) + return admission.Errored(http.StatusBadRequest, err) + } + + return admission.ValidationResponse(true, "allowed") +} + +func validate(obj *appsv1alpha1.EphemeralJob) error { + ecs, err := convertor.ConvertEphemeralContainer(obj.Spec.Template.EphemeralContainers) + if err != nil { + return err + } + // don't validate EphemeralContainer TargetContainerName + allErrs := validateEphemeralContainers(ecs, field.NewPath("ephemeralContainers"), validation.PodValidationOptions{}) + return allErrs.ToAggregate() +} diff --git a/pkg/webhook/ephemeraljob/validating/ephemeraljob_create_update_handler_test.go b/pkg/webhook/ephemeraljob/validating/ephemeraljob_create_update_handler_test.go new file mode 100644 index 0000000000..f2683d7b2f --- /dev/null +++ b/pkg/webhook/ephemeraljob/validating/ephemeraljob_create_update_handler_test.go @@ -0,0 +1,170 @@ +package validating + +import ( + "context" + "fmt" + "net/http" + "reflect" + "testing" + + admissionv1 "k8s.io/api/admission/v1" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + alpha1 "github.com/openkruise/kruise/apis/apps/v1alpha1" +) + +var scheme = runtime.NewScheme() + +func init() { + scheme = runtime.NewScheme() + _ = alpha1.AddToScheme(scheme) +} + +func getFailedJSON() string { + return `{ + "apiVersion": "apps.kruise.io/v1alpha1", + "kind": "EphemeralJob", + "metadata": { + "creationTimestamp": "2024-05-09T08:29:50Z", + "generation": 1, + "name": "ephermeraljob-sample", + "namespace": "test" + }, + "spec": { + "parallelism": 1, + "replicas": 1, + "selector": { + "matchLabels": { + "app": "test-2" + } + }, + "template": { + "ephemeralContainers": { + "ephemeralContainerCommon": { + "image": "busybox", + "imagePullPolicy": "IfNotPresent", + "name": "debugger", + "securityContext": { + "capabilities": { + "add": [ + "SYS_ADMIN", + "NET_ADMIN" + ] + } + }, + "terminationMessagePolicy": "File" + }, + "targetContainerName": "test" + } + }, + "ttlSecondsAfterFinished": 1800 + } +}` +} + +func getOKJSON(targetContainerName string) string { + return fmt.Sprintf(`{ + "apiVersion": "apps.kruise.io/v1alpha1", + "kind": "EphemeralJob", + "metadata": { + "name": "ephermeraljob-sample", + "namespace": "test" + }, + "spec": { + "parallelism": 1, + "replicas": 1, + "selector": { + "matchLabels": { + "app": "test-2" + } + }, + "template": { + "ephemeralContainers": [{ + "image": "busybox", + "imagePullPolicy": "IfNotPresent", + "name": "debugger", + "securityContext": { + "capabilities": { + "add": [ + "SYS_ADMIN", + "NET_ADMIN" + ] + } + }, + "terminationMessagePolicy": "File", + "targetContainerName": "%v" + }] + }, + "ttlSecondsAfterFinished": 1800 + } +}`, targetContainerName) +} + +func TestEphemeralJobCreateUpdateHandler_Handle(t *testing.T) { + type args struct { + req admission.Request + } + tests := []struct { + name string + args args + wantOK bool + }{ + { + name: "failed case", + wantOK: false, + args: args{ + req: admission.Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + Operation: admissionv1.Create, + Object: runtime.RawExtension{ + Raw: []byte(getFailedJSON()), + }, + }, + }, + }, + }, + { + name: "ok case with empty targetContainerName", + wantOK: true, + args: args{ + req: admission.Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + Operation: admissionv1.Create, + Object: runtime.RawExtension{ + Raw: []byte(getOKJSON("")), + }, + }, + }, + }, + }, + { + name: "ok case with targetContainerName", + wantOK: true, + args: args{ + req: admission.Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + Operation: admissionv1.Create, + Object: runtime.RawExtension{ + Raw: []byte(getOKJSON("test")), + }, + }, + }, + }, + }, + } + + d := admission.NewDecoder(scheme) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + h := &EphemeralJobCreateUpdateHandler{ + Decoder: d, + } + if got := h.Handle(context.TODO(), tt.args.req); !reflect.DeepEqual(got.Allowed, tt.wantOK) { + t.Errorf("Handle() = %v, want %v", got.Result.Code == http.StatusOK, tt.wantOK) + } else if !got.Allowed { + t.Log(got.Result.Message) + } + }) + } +} diff --git a/pkg/webhook/ephemeraljob/validating/validation.go b/pkg/webhook/ephemeraljob/validating/validation.go new file mode 100644 index 0000000000..8f1b7abbb7 --- /dev/null +++ b/pkg/webhook/ephemeraljob/validating/validation.go @@ -0,0 +1,215 @@ +package validating + +import ( + "reflect" + "strings" + "unicode" + "unicode/utf8" + + "k8s.io/apimachinery/pkg/util/sets" + utilvalidation "k8s.io/apimachinery/pkg/util/validation" + "k8s.io/apimachinery/pkg/util/validation/field" + "k8s.io/kubernetes/pkg/apis/core" + "k8s.io/kubernetes/pkg/apis/core/validation" +) + +// reference: https://github.com/kubernetes/kubernetes/blob/0d90d1ffa5e87dfc4d3098da7f281351c7ff1972/pkg/apis/core/validation/validation.go\#L3171 +var supportedPullPolicies = sets.NewString(string(core.PullAlways), string(core.PullIfNotPresent), string(core.PullNever)) +var supportedPortProtocols = sets.NewString(string(core.ProtocolTCP), string(core.ProtocolUDP), string(core.ProtocolSCTP)) + +var allowedEphemeralContainerFields = map[string]bool{ + "Name": true, + "Image": true, + "Command": true, + "Args": true, + "WorkingDir": true, + "Ports": false, + "EnvFrom": true, + "Env": true, + "Resources": false, + "VolumeMounts": true, + "VolumeDevices": true, + "LivenessProbe": false, + "ReadinessProbe": false, + "StartupProbe": false, + "Lifecycle": false, + "TerminationMessagePath": true, + "TerminationMessagePolicy": true, + "ImagePullPolicy": true, + "SecurityContext": true, + "Stdin": true, + "StdinOnce": true, + "TTY": true, +} + +func validateContainerPorts(ports []core.ContainerPort, fldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + + allNames := sets.String{} + for i, port := range ports { + idxPath := fldPath.Index(i) + if len(port.Name) > 0 { + if msgs := utilvalidation.IsValidPortName(port.Name); len(msgs) != 0 { + for i = range msgs { + allErrs = append(allErrs, field.Invalid(idxPath.Child("name"), port.Name, msgs[i])) + } + } else if allNames.Has(port.Name) { + allErrs = append(allErrs, field.Duplicate(idxPath.Child("name"), port.Name)) + } else { + allNames.Insert(port.Name) + } + } + if port.ContainerPort == 0 { + allErrs = append(allErrs, field.Required(idxPath.Child("containerPort"), "")) + } else { + for _, msg := range utilvalidation.IsValidPortNum(int(port.ContainerPort)) { + allErrs = append(allErrs, field.Invalid(idxPath.Child("containerPort"), port.ContainerPort, msg)) + } + } + if port.HostPort != 0 { + for _, msg := range utilvalidation.IsValidPortNum(int(port.HostPort)) { + allErrs = append(allErrs, field.Invalid(idxPath.Child("hostPort"), port.HostPort, msg)) + } + } + if len(port.Protocol) == 0 { + allErrs = append(allErrs, field.Required(idxPath.Child("protocol"), "")) + } else if !supportedPortProtocols.Has(string(port.Protocol)) { + allErrs = append(allErrs, field.NotSupported(idxPath.Child("protocol"), port.Protocol, supportedPortProtocols.List())) + } + } + return allErrs +} + +func validatePullPolicy(policy core.PullPolicy, fldPath *field.Path) field.ErrorList { + allErrors := field.ErrorList{} + + switch policy { + case core.PullAlways, core.PullIfNotPresent, core.PullNever: + break + case "": + allErrors = append(allErrors, field.Required(fldPath, "")) + default: + allErrors = append(allErrors, field.NotSupported(fldPath, policy, supportedPullPolicies.List())) + } + return allErrors +} + +// validateContainerCommon applies validation common to all container types. It's called by regular, init, and ephemeral +// container list validation to require a properly formatted name, image, etc. +func validateContainerCommon(ctr *core.Container, path *field.Path, opts validation.PodValidationOptions) field.ErrorList { + var allErrs field.ErrorList + + namePath := path.Child("name") + if len(ctr.Name) == 0 { + allErrs = append(allErrs, field.Required(namePath, "")) + } else { + allErrs = append(allErrs, validation.ValidateDNS1123Label(ctr.Name, namePath)...) + } + + // TODO: do not validate leading and trailing whitespace to preserve backward compatibility. + // for example: https://github.com/openshift/origin/issues/14659 image = " " is special token in pod template + // others may have done similar + if len(ctr.Image) == 0 { + allErrs = append(allErrs, field.Required(path.Child("image"), "")) + } + + switch ctr.TerminationMessagePolicy { + case core.TerminationMessageReadFile, core.TerminationMessageFallbackToLogsOnError: + case "": + allErrs = append(allErrs, field.Required(path.Child("terminationMessagePolicy"), "")) + default: + supported := []string{ + string(core.TerminationMessageReadFile), + string(core.TerminationMessageFallbackToLogsOnError), + } + allErrs = append(allErrs, field.NotSupported(path.Child("terminationMessagePolicy"), ctr.TerminationMessagePolicy, supported)) + } + + allErrs = append(allErrs, validateContainerPorts(ctr.Ports, path.Child("ports"))...) + allErrs = append(allErrs, validation.ValidateEnv(ctr.Env, path.Child("env"), opts)...) + allErrs = append(allErrs, validation.ValidateEnvFrom(ctr.EnvFrom, path.Child("envFrom"))...) + allErrs = append(allErrs, validatePullPolicy(ctr.ImagePullPolicy, path.Child("imagePullPolicy"))...) + allErrs = append(allErrs, validation.ValidateSecurityContext(ctr.SecurityContext, path.Child("securityContext"))...) + return allErrs +} + +// validateContainerOnlyForPod does pod-only (i.e. not pod template) validation for a single container. +// This is called by validateContainersOnlyForPod and validateEphemeralContainers directly. +func validateContainerOnlyForPod(ctr *core.Container, path *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + if len(ctr.Image) != len(strings.TrimSpace(ctr.Image)) { + allErrs = append(allErrs, field.Invalid(path.Child("image"), ctr.Image, "must not have leading or trailing whitespace")) + } + return allErrs +} + +// ValidateFieldAcceptList checks that only allowed fields are set. +// The value must be a struct (not a pointer to a struct!). +func validateFieldAllowList(value interface{}, allowedFields map[string]bool, errorText string, fldPath *field.Path) field.ErrorList { + var allErrs field.ErrorList + + reflectType, reflectValue := reflect.TypeOf(value), reflect.ValueOf(value) + for i := 0; i < reflectType.NumField(); i++ { + f := reflectType.Field(i) + if allowedFields[f.Name] { + continue + } + + // Compare the value of this field to its zero value to determine if it has been set + if !reflect.DeepEqual(reflectValue.Field(i).Interface(), reflect.Zero(f.Type).Interface()) { + r, n := utf8.DecodeRuneInString(f.Name) + lcName := string(unicode.ToLower(r)) + f.Name[n:] + allErrs = append(allErrs, field.Forbidden(fldPath.Child(lcName), errorText)) + } + } + + return allErrs +} + +// validateEphemeralContainers is called by pod spec and template validation to validate the list of ephemeral containers. +// Note that this is called for pod template even though ephemeral containers aren't allowed in pod templates. +func validateEphemeralContainers(ephemeralContainers []core.EphemeralContainer, fldPath *field.Path, opts validation.PodValidationOptions) field.ErrorList { + var allErrs field.ErrorList + + if len(ephemeralContainers) == 0 { + return allErrs + } + + allNames := sets.String{} + + for i, ec := range ephemeralContainers { + idxPath := fldPath.Index(i) + + c := (*core.Container)(&ec.EphemeralContainerCommon) + allErrs = append(allErrs, validateContainerCommon(c, idxPath, opts)...) + // Ephemeral containers don't need looser constraints for pod templates, so it's convenient to apply both validations + // here where we've already converted EphemeralContainerCommon to Container. + allErrs = append(allErrs, validateContainerOnlyForPod(c, idxPath)...) + + // Ephemeral containers must have a name unique across all container types. + if allNames.Has(ec.Name) { + allErrs = append(allErrs, field.Duplicate(idxPath.Child("name"), ec.Name)) + } else { + allNames.Insert(ec.Name) + } + + // Ephemeral containers should not be relied upon for fundamental pod services, so fields such as + // Lifecycle, probes, resources and ports should be disallowed. This is implemented as a list + // of allowed fields so that new fields will be given consideration prior to inclusion in ephemeral containers. + allErrs = append(allErrs, validateFieldAllowList(ec.EphemeralContainerCommon, allowedEphemeralContainerFields, "cannot be set for an Ephemeral Container", idxPath)...) + + // VolumeMount subpaths have the potential to leak resources since they're implemented with bind mounts + // that aren't cleaned up until the pod exits. Since they also imply that the container is being used + // as part of the workload, they're disallowed entirely. + for i, vm := range ec.VolumeMounts { + if vm.SubPath != "" { + allErrs = append(allErrs, field.Forbidden(idxPath.Child("volumeMounts").Index(i).Child("subPath"), "cannot be set for an Ephemeral Container")) + } + if vm.SubPathExpr != "" { + allErrs = append(allErrs, field.Forbidden(idxPath.Child("volumeMounts").Index(i).Child("subPathExpr"), "cannot be set for an Ephemeral Container")) + } + } + } + + return allErrs +} diff --git a/pkg/webhook/ephemeraljob/validating/validation_test.go b/pkg/webhook/ephemeraljob/validating/validation_test.go new file mode 100644 index 0000000000..43ae7fc6bc --- /dev/null +++ b/pkg/webhook/ephemeraljob/validating/validation_test.go @@ -0,0 +1,394 @@ +package validating + +import ( + "fmt" + "runtime" + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + "k8s.io/apimachinery/pkg/api/resource" + "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/apimachinery/pkg/util/validation/field" + "k8s.io/kubernetes/pkg/apis/core" + "k8s.io/kubernetes/pkg/apis/core/validation" +) + +var ( + containerRestartPolicyAlways = core.ContainerRestartPolicyAlways + containerRestartPolicyOnFailure = core.ContainerRestartPolicy("OnFailure") + containerRestartPolicyNever = core.ContainerRestartPolicy("Never") + containerRestartPolicyInvalid = core.ContainerRestartPolicy("invalid") + containerRestartPolicyEmpty = core.ContainerRestartPolicy("") +) + +func line() string { + _, _, line, ok := runtime.Caller(1) + var s string + if ok { + s = fmt.Sprintf("%d", line) + } else { + s = "" + } + return s +} + +func prettyErrorList(errs field.ErrorList) string { + var s string + for _, e := range errs { + s += fmt.Sprintf("\t%s\n", e) + } + return s +} + +func TestValidateEphemeralContainers(t *testing.T) { + // Success Cases + for title, ephemeralContainers := range map[string][]core.EphemeralContainer{ + "Empty Ephemeral Containers": {}, + "Single Container": { + {EphemeralContainerCommon: core.EphemeralContainerCommon{Name: "debug", Image: "image", ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: "File"}}, + }, + "Multiple Containers": { + {EphemeralContainerCommon: core.EphemeralContainerCommon{Name: "debug1", Image: "image", ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: "File"}}, + {EphemeralContainerCommon: core.EphemeralContainerCommon{Name: "debug2", Image: "image", ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: "File"}}, + }, + "Single Container with Target": {{ + EphemeralContainerCommon: core.EphemeralContainerCommon{Name: "debug", Image: "image", ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: "File"}, + TargetContainerName: "ctr", + }}, + "All allowed fields": {{ + EphemeralContainerCommon: core.EphemeralContainerCommon{ + + Name: "debug", + Image: "image", + Command: []string{"bash"}, + Args: []string{"bash"}, + WorkingDir: "/", + EnvFrom: []core.EnvFromSource{{ + ConfigMapRef: &core.ConfigMapEnvSource{ + LocalObjectReference: core.LocalObjectReference{Name: "dummy"}, + Optional: &[]bool{true}[0], + }, + }}, + Env: []core.EnvVar{ + {Name: "TEST", Value: "TRUE"}, + }, + VolumeMounts: []core.VolumeMount{ + {Name: "vol", MountPath: "/vol"}, + }, + VolumeDevices: []core.VolumeDevice{ + {Name: "blk", DevicePath: "/dev/block"}, + }, + TerminationMessagePath: "/dev/termination-log", + TerminationMessagePolicy: "File", + ImagePullPolicy: "IfNotPresent", + SecurityContext: &core.SecurityContext{ + Capabilities: &core.Capabilities{ + Add: []core.Capability{"SYS_ADMIN"}, + }, + }, + Stdin: true, + StdinOnce: true, + TTY: true, + }, + }}, + } { + if errs := validateEphemeralContainers(ephemeralContainers, field.NewPath("ephemeralContainers"), validation.PodValidationOptions{}); len(errs) != 0 { + t.Errorf("expected success for '%s' but got errors: %v", title, errs) + } + } + + // Failure Cases + tcs := []struct { + title, line string + ephemeralContainers []core.EphemeralContainer + expectedErrors field.ErrorList + }{{ + "Name Collision with EphemeralContainers", + line(), + []core.EphemeralContainer{ + {EphemeralContainerCommon: core.EphemeralContainerCommon{Name: "debug1", Image: "image", ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: "File"}}, + {EphemeralContainerCommon: core.EphemeralContainerCommon{Name: "debug1", Image: "image", ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: "File"}}, + }, + field.ErrorList{{Type: field.ErrorTypeDuplicate, Field: "ephemeralContainers[1].name"}}, + }, { + "empty Container", + line(), + []core.EphemeralContainer{ + {EphemeralContainerCommon: core.EphemeralContainerCommon{}}, + }, + field.ErrorList{ + {Type: field.ErrorTypeRequired, Field: "ephemeralContainers[0].name"}, + {Type: field.ErrorTypeRequired, Field: "ephemeralContainers[0].image"}, + {Type: field.ErrorTypeRequired, Field: "ephemeralContainers[0].terminationMessagePolicy"}, + {Type: field.ErrorTypeRequired, Field: "ephemeralContainers[0].imagePullPolicy"}, + }, + }, { + "empty Container Name", + line(), + []core.EphemeralContainer{ + {EphemeralContainerCommon: core.EphemeralContainerCommon{Name: "", Image: "image", ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: "File"}}, + }, + field.ErrorList{{Type: field.ErrorTypeRequired, Field: "ephemeralContainers[0].name"}}, + }, { + "whitespace padded image name", + line(), + []core.EphemeralContainer{ + {EphemeralContainerCommon: core.EphemeralContainerCommon{Name: "debug", Image: " image", ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: "File"}}, + }, + field.ErrorList{{Type: field.ErrorTypeInvalid, Field: "ephemeralContainers[0].image"}}, + }, { + "invalid image pull policy", + line(), + []core.EphemeralContainer{ + {EphemeralContainerCommon: core.EphemeralContainerCommon{Name: "debug", Image: "image", ImagePullPolicy: "PullThreeTimes", TerminationMessagePolicy: "File"}}, + }, + field.ErrorList{{Type: field.ErrorTypeNotSupported, Field: "ephemeralContainers[0].imagePullPolicy"}}, + }, { + "Container uses disallowed field: Lifecycle", + line(), + []core.EphemeralContainer{{ + EphemeralContainerCommon: core.EphemeralContainerCommon{ + Name: "debug", + Image: "image", + ImagePullPolicy: "IfNotPresent", + TerminationMessagePolicy: "File", + Lifecycle: &core.Lifecycle{ + PreStop: &core.LifecycleHandler{ + Exec: &core.ExecAction{Command: []string{"ls", "-l"}}, + }, + }, + }, + }}, + field.ErrorList{{Type: field.ErrorTypeForbidden, Field: "ephemeralContainers[0].lifecycle"}}, + }, { + "Container uses disallowed field: LivenessProbe", + line(), + []core.EphemeralContainer{{ + EphemeralContainerCommon: core.EphemeralContainerCommon{ + Name: "debug", + Image: "image", + ImagePullPolicy: "IfNotPresent", + TerminationMessagePolicy: "File", + LivenessProbe: &core.Probe{ + ProbeHandler: core.ProbeHandler{ + TCPSocket: &core.TCPSocketAction{Port: intstr.FromInt32(80)}, + }, + SuccessThreshold: 1, + }, + }, + }}, + field.ErrorList{{Type: field.ErrorTypeForbidden, Field: "ephemeralContainers[0].livenessProbe"}}, + }, { + "Container uses disallowed field: Ports", + line(), + []core.EphemeralContainer{{ + EphemeralContainerCommon: core.EphemeralContainerCommon{ + Name: "debug", + Image: "image", + ImagePullPolicy: "IfNotPresent", + TerminationMessagePolicy: "File", + Ports: []core.ContainerPort{ + {Protocol: "TCP", ContainerPort: 80}, + }, + }, + }}, + field.ErrorList{{Type: field.ErrorTypeForbidden, Field: "ephemeralContainers[0].ports"}}, + }, { + "Container uses disallowed field: ReadinessProbe", + line(), + []core.EphemeralContainer{{ + EphemeralContainerCommon: core.EphemeralContainerCommon{ + Name: "debug", + Image: "image", + ImagePullPolicy: "IfNotPresent", + TerminationMessagePolicy: "File", + ReadinessProbe: &core.Probe{ + ProbeHandler: core.ProbeHandler{ + TCPSocket: &core.TCPSocketAction{Port: intstr.FromInt32(80)}, + }, + }, + }, + }}, + field.ErrorList{{Type: field.ErrorTypeForbidden, Field: "ephemeralContainers[0].readinessProbe"}}, + }, { + "Container uses disallowed field: StartupProbe", + line(), + []core.EphemeralContainer{{ + EphemeralContainerCommon: core.EphemeralContainerCommon{ + Name: "debug", + Image: "image", + ImagePullPolicy: "IfNotPresent", + TerminationMessagePolicy: "File", + StartupProbe: &core.Probe{ + ProbeHandler: core.ProbeHandler{ + TCPSocket: &core.TCPSocketAction{Port: intstr.FromInt32(80)}, + }, + SuccessThreshold: 1, + }, + }, + }}, + field.ErrorList{{Type: field.ErrorTypeForbidden, Field: "ephemeralContainers[0].startupProbe"}}, + }, { + "Container uses disallowed field: Resources", + line(), + []core.EphemeralContainer{{ + EphemeralContainerCommon: core.EphemeralContainerCommon{ + Name: "debug", + Image: "image", + ImagePullPolicy: "IfNotPresent", + TerminationMessagePolicy: "File", + Resources: core.ResourceRequirements{ + Limits: core.ResourceList{ + core.ResourceCPU: resource.MustParse("10"), + }, + }, + }, + }}, + field.ErrorList{{Type: field.ErrorTypeForbidden, Field: "ephemeralContainers[0].resources"}}, + }, { + "Container uses disallowed field: VolumeMount.SubPath", + line(), + []core.EphemeralContainer{{ + EphemeralContainerCommon: core.EphemeralContainerCommon{ + Name: "debug", + Image: "image", + ImagePullPolicy: "IfNotPresent", + TerminationMessagePolicy: "File", + VolumeMounts: []core.VolumeMount{ + {Name: "vol", MountPath: "/vol"}, + {Name: "vol", MountPath: "/volsub", SubPath: "foo"}, + }, + }, + }}, + field.ErrorList{{Type: field.ErrorTypeForbidden, Field: "ephemeralContainers[0].volumeMounts[1].subPath"}}, + }, { + "Container uses disallowed field: VolumeMount.SubPathExpr", + line(), + []core.EphemeralContainer{{ + EphemeralContainerCommon: core.EphemeralContainerCommon{ + Name: "debug", + Image: "image", + ImagePullPolicy: "IfNotPresent", + TerminationMessagePolicy: "File", + VolumeMounts: []core.VolumeMount{ + {Name: "vol", MountPath: "/vol"}, + {Name: "vol", MountPath: "/volsub", SubPathExpr: "$(POD_NAME)"}, + }, + }, + }}, + field.ErrorList{{Type: field.ErrorTypeForbidden, Field: "ephemeralContainers[0].volumeMounts[1].subPathExpr"}}, + }, { + "Disallowed field with other errors should only return a single Forbidden", + line(), + []core.EphemeralContainer{{ + EphemeralContainerCommon: core.EphemeralContainerCommon{ + Name: "debug", + Image: "image", + ImagePullPolicy: "IfNotPresent", + TerminationMessagePolicy: "File", + Lifecycle: &core.Lifecycle{ + PreStop: &core.LifecycleHandler{ + Exec: &core.ExecAction{Command: []string{}}, + }, + }, + }, + }}, + field.ErrorList{{Type: field.ErrorTypeForbidden, Field: "ephemeralContainers[0].lifecycle"}}, + }, { + "Container uses disallowed field: ResizePolicy", + line(), + []core.EphemeralContainer{{ + EphemeralContainerCommon: core.EphemeralContainerCommon{ + Name: "resources-resize-policy", + Image: "image", + ImagePullPolicy: "IfNotPresent", + TerminationMessagePolicy: "File", + ResizePolicy: []core.ContainerResizePolicy{ + {ResourceName: "cpu", RestartPolicy: "NotRequired"}, + }, + }, + }}, + field.ErrorList{{Type: field.ErrorTypeForbidden, Field: "ephemeralContainers[0].resizePolicy"}}, + }, { + "Forbidden RestartPolicy: Always", + line(), + []core.EphemeralContainer{{ + EphemeralContainerCommon: core.EphemeralContainerCommon{ + Name: "foo", + Image: "image", + ImagePullPolicy: "IfNotPresent", + TerminationMessagePolicy: "File", + RestartPolicy: &containerRestartPolicyAlways, + }, + }}, + field.ErrorList{{Type: field.ErrorTypeForbidden, Field: "ephemeralContainers[0].restartPolicy"}}, + }, { + "Forbidden RestartPolicy: OnFailure", + line(), + []core.EphemeralContainer{{ + EphemeralContainerCommon: core.EphemeralContainerCommon{ + Name: "foo", + Image: "image", + ImagePullPolicy: "IfNotPresent", + TerminationMessagePolicy: "File", + RestartPolicy: &containerRestartPolicyOnFailure, + }, + }}, + field.ErrorList{{Type: field.ErrorTypeForbidden, Field: "ephemeralContainers[0].restartPolicy"}}, + }, { + "Forbidden RestartPolicy: Never", + line(), + []core.EphemeralContainer{{ + EphemeralContainerCommon: core.EphemeralContainerCommon{ + Name: "foo", + Image: "image", + ImagePullPolicy: "IfNotPresent", + TerminationMessagePolicy: "File", + RestartPolicy: &containerRestartPolicyNever, + }, + }}, + field.ErrorList{{Type: field.ErrorTypeForbidden, Field: "ephemeralContainers[0].restartPolicy"}}, + }, { + "Forbidden RestartPolicy: invalid", + line(), + []core.EphemeralContainer{{ + EphemeralContainerCommon: core.EphemeralContainerCommon{ + Name: "foo", + Image: "image", + ImagePullPolicy: "IfNotPresent", + TerminationMessagePolicy: "File", + RestartPolicy: &containerRestartPolicyInvalid, + }, + }}, + field.ErrorList{{Type: field.ErrorTypeForbidden, Field: "ephemeralContainers[0].restartPolicy"}}, + }, { + "Forbidden RestartPolicy: empty", + line(), + []core.EphemeralContainer{{ + EphemeralContainerCommon: core.EphemeralContainerCommon{ + Name: "foo", + Image: "image", + ImagePullPolicy: "IfNotPresent", + TerminationMessagePolicy: "File", + RestartPolicy: &containerRestartPolicyEmpty, + }, + }}, + field.ErrorList{{Type: field.ErrorTypeForbidden, Field: "ephemeralContainers[0].restartPolicy"}}, + }, + } + + for _, tc := range tcs { + t.Run(tc.title+"__@L"+tc.line, func(t *testing.T) { + errs := validateEphemeralContainers(tc.ephemeralContainers, field.NewPath("ephemeralContainers"), validation.PodValidationOptions{}) + if len(errs) == 0 { + t.Fatal("expected error but received none") + } + + if diff := cmp.Diff(tc.expectedErrors, errs, cmpopts.IgnoreFields(field.Error{}, "BadValue", "Detail")); diff != "" { + t.Errorf("unexpected diff in errors (-want, +got):\n%s", diff) + t.Errorf("INFO: all errors:\n%s", prettyErrorList(errs)) + } + }) + } +} diff --git a/pkg/webhook/ephemeraljob/validating/webhooks.go b/pkg/webhook/ephemeraljob/validating/webhooks.go new file mode 100644 index 0000000000..cc880bb339 --- /dev/null +++ b/pkg/webhook/ephemeraljob/validating/webhooks.go @@ -0,0 +1,35 @@ +/* +Copyright 2021 The Kruise Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package validating + +import ( + "sigs.k8s.io/controller-runtime/pkg/manager" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + "github.com/openkruise/kruise/pkg/webhook/types" +) + +// +kubebuilder:webhook:path=/validate-apps-kruise-io-v1alpha1-ephemeraljob,mutating=false,failurePolicy=fail,sideEffects=None,admissionReviewVersions=v1;v1beta1,groups=apps.kruise.io,resources=ephemeraljobs,verbs=create;update,versions=v1alpha1,name=vephemeraljobs.kb.io + +var ( + // HandlerGetterMap contains admission webhook handlers + HandlerGetterMap = map[string]types.HandlerGetter{ + "validate-apps-kruise-io-v1alpha1-ephemeraljob": func(mgr manager.Manager) admission.Handler { + return NewHandler(mgr) + }, + } +) diff --git a/pkg/webhook/util/convertor/util.go b/pkg/webhook/util/convertor/util.go index 85d86979ae..65c3d57fc9 100644 --- a/pkg/webhook/util/convertor/util.go +++ b/pkg/webhook/util/convertor/util.go @@ -19,12 +19,13 @@ package convertor import ( "strconv" - "github.com/openkruise/kruise/apis/apps/defaults" v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/apimachinery/pkg/util/validation" "k8s.io/kubernetes/pkg/apis/core" corev1 "k8s.io/kubernetes/pkg/apis/core/v1" + + "github.com/openkruise/kruise/apis/apps/defaults" ) func ConvertPodTemplateSpec(template *v1.PodTemplateSpec) (*core.PodTemplateSpec, error) { @@ -57,6 +58,18 @@ func ConvertCoreVolumes(volumes []v1.Volume) ([]core.Volume, error) { return coreVolumes, nil } +func ConvertEphemeralContainer(ecs []v1.EphemeralContainer) ([]core.EphemeralContainer, error) { + coreEphemeralContainers := []core.EphemeralContainer{} + for _, ec := range ecs { + coreEC := core.EphemeralContainer{} + if err := corev1.Convert_v1_EphemeralContainer_To_core_EphemeralContainer(&ec, &coreEC, nil); err != nil { + return nil, err + } + coreEphemeralContainers = append(coreEphemeralContainers, coreEC) + } + return coreEphemeralContainers, nil +} + func GetPercentValue(intOrStringValue intstr.IntOrString) (int, bool) { if intOrStringValue.Type != intstr.String { return 0, false From 145a9af1df545f52620d115e01169a0fe3a46dbd Mon Sep 17 00:00:00 2001 From: Abner Date: Wed, 5 Jun 2024 12:10:36 +0800 Subject: [PATCH 5/6] fix ut error in some machines without docker auth info (#1640) Signed-off-by: Abner-1 --- .../criruntime/imageruntime/helpers_test.go | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/pkg/daemon/criruntime/imageruntime/helpers_test.go b/pkg/daemon/criruntime/imageruntime/helpers_test.go index ef2119d01a..a4a8039845 100644 --- a/pkg/daemon/criruntime/imageruntime/helpers_test.go +++ b/pkg/daemon/criruntime/imageruntime/helpers_test.go @@ -30,7 +30,8 @@ func TestMatchRegistryAuths(t *testing.T) { name string Image string GetSecrets func() []v1.Secret - Expect int + // fix issue https://github.com/openkruise/kruise/issues/1583 + ExpectMinValue int }{ { name: "test1", @@ -44,7 +45,7 @@ func TestMatchRegistryAuths(t *testing.T) { } return []v1.Secret{demo} }, - Expect: 1, + ExpectMinValue: 1, }, { name: "test2", @@ -58,7 +59,7 @@ func TestMatchRegistryAuths(t *testing.T) { } return []v1.Secret{demo} }, - Expect: 1, + ExpectMinValue: 1, }, { name: "test3", @@ -72,7 +73,7 @@ func TestMatchRegistryAuths(t *testing.T) { } return []v1.Secret{demo} }, - Expect: 1, + ExpectMinValue: 1, }, { name: "test4", @@ -86,7 +87,7 @@ func TestMatchRegistryAuths(t *testing.T) { } return []v1.Secret{demo} }, - Expect: 2, + ExpectMinValue: 1, }, { name: "test5", @@ -100,7 +101,7 @@ func TestMatchRegistryAuths(t *testing.T) { } return []v1.Secret{demo} }, - Expect: 0, + ExpectMinValue: 0, }, } for _, cs := range cases { @@ -113,7 +114,7 @@ func TestMatchRegistryAuths(t *testing.T) { if err != nil { t.Fatalf("convertToRegistryAuths failed: %s", err.Error()) } - if len(infos) != cs.Expect { + if len(infos) < cs.ExpectMinValue { t.Fatalf("convertToRegistryAuths failed") } }) From 5ea03f19bed231aaf2d1456013d86559afc8a802 Mon Sep 17 00:00:00 2001 From: Kuromesi <87558945+Kuromesi@users.noreply.github.com> Date: Tue, 11 Jun 2024 09:39:42 +0800 Subject: [PATCH 6/6] add support for credential provider plugin (#1383) Signed-off-by: Kuromesi --- Makefile | 2 + cmd/daemon/main.go | 27 +++- pkg/daemon/criruntime/imageruntime/cri.go | 44 ++--- pkg/daemon/criruntime/imageruntime/docker.go | 36 ++--- .../imageruntime/fake_plugin/main.go | 152 ++++++++++++++++++ .../fake_plugin/plugin-config.yaml | 9 ++ .../criruntime/imageruntime/helpers_test.go | 21 +++ pkg/daemon/criruntime/imageruntime/pouch.go | 36 ++--- pkg/util/secret/parse.go | 12 +- 9 files changed, 277 insertions(+), 62 deletions(-) create mode 100644 pkg/daemon/criruntime/imageruntime/fake_plugin/main.go create mode 100644 pkg/daemon/criruntime/imageruntime/fake_plugin/plugin-config.yaml diff --git a/Makefile b/Makefile index b4d371610b..76f8e87bd9 100644 --- a/Makefile +++ b/Makefile @@ -48,7 +48,9 @@ lint: golangci-lint ## Run golangci-lint against code. test: generate fmt vet manifests envtest ## Run tests echo $(ENVTEST) + go build -o pkg/daemon/criruntime/imageruntime/fake_plugin/fake-credential-plugin pkg/daemon/criruntime/imageruntime/fake_plugin/main.go && chmod +x pkg/daemon/criruntime/imageruntime/fake_plugin/fake-credential-plugin KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) -p path)" go test ./pkg/... -coverprofile cover.out + rm pkg/daemon/criruntime/imageruntime/fake_plugin/fake-credential-plugin coverage-report: ## Generate cover.html from cover.out go tool cover -html=cover.out -o cover.html diff --git a/cmd/daemon/main.go b/cmd/daemon/main.go index 4257a667e8..a11ee02e9b 100644 --- a/cmd/daemon/main.go +++ b/cmd/daemon/main.go @@ -17,6 +17,10 @@ limitations under the License. package main import ( + "os" + + "k8s.io/kubernetes/pkg/credentialprovider/plugin" + "flag" "math/rand" "net/http" @@ -34,12 +38,15 @@ import ( "github.com/openkruise/kruise/pkg/daemon" "github.com/openkruise/kruise/pkg/features" utilfeature "github.com/openkruise/kruise/pkg/util/feature" + "github.com/openkruise/kruise/pkg/util/secret" ) var ( - bindAddr = flag.String("addr", ":10221", "The address the metric endpoint and healthz binds to.") - pprofAddr = flag.String("pprof-addr", ":10222", "The address the pprof binds to.") - enablePprof = flag.Bool("enable-pprof", true, "Enable pprof for daemon.") + bindAddr = flag.String("addr", ":10221", "The address the metric endpoint and healthz binds to.") + pprofAddr = flag.String("pprof-addr", ":10222", "The address the pprof binds to.") + enablePprof = flag.Bool("enable-pprof", true, "Enable pprof for daemon.") + pluginConfigFile = flag.String("plugin-config-file", "/kruise/CredentialProviderPlugin.yaml", "The path of plugin config file.") + pluginBinDir = flag.String("plugin-bin-dir", "/kruise/plugins", "The path of directory of plugin binaries.") ) func main() { @@ -68,6 +75,20 @@ func main() { if err != nil { klog.Fatalf("Failed to new daemon: %v", err) } + + if _, err := os.Stat(*pluginConfigFile); err == nil { + err = plugin.RegisterCredentialProviderPlugins(*pluginConfigFile, *pluginBinDir) + if err != nil { + klog.Errorf("Failed to register credential provider plugins: %v", err) + } + } else if os.IsNotExist(err) { + klog.Infof("No plugin config file found, skipping: %s", *pluginConfigFile) + } else { + klog.Errorf("Failed to check plugin config file: %v", err) + } + // make sure the new docker key ring is made and set after the credential plugins are registered + secret.MakeAndSetKeyring() + if err := d.Run(ctx); err != nil { klog.Fatalf("Failed to start daemon: %v", err) } diff --git a/pkg/daemon/criruntime/imageruntime/cri.go b/pkg/daemon/criruntime/imageruntime/cri.go index 0f303a2606..957212d299 100644 --- a/pkg/daemon/criruntime/imageruntime/cri.go +++ b/pkg/daemon/criruntime/imageruntime/cri.go @@ -131,31 +131,31 @@ func (c *commonCRIImageService) pullImageV1(ctx context.Context, imageName, tag // for some runtime implementations. pullImageReq.SandboxConfig.Annotations[pullingImageSandboxConfigAnno] = "kruise-daemon" - if len(pullSecrets) > 0 { - var authInfos []daemonutil.AuthInfo - authInfos, err = secret.ConvertToRegistryAuths(pullSecrets, repoToPull) - if err == nil { - var pullErrs []error - for _, authInfo := range authInfos { - var pullErr error - klog.V(5).Infof("Pull image %v:%v with user %v", imageName, tag, authInfo.Username) - pullImageReq.Auth = &runtimeapi.AuthConfig{ - Username: authInfo.Username, - Password: authInfo.Password, - } - _, pullErr = c.criImageClient.PullImage(ctx, pullImageReq) - if pullErr == nil { - pipeW.CloseWithError(io.EOF) - return newImagePullStatusReader(pipeR), nil - } - klog.Warningf("Failed to pull image %v:%v with user %v, err %v", imageName, tag, authInfo.Username, pullErr) - pullErrs = append(pullErrs, pullErr) - + var authInfos []daemonutil.AuthInfo + authInfos, err = secret.ConvertToRegistryAuths(pullSecrets, repoToPull) + if err == nil { + var pullErrs []error + for _, authInfo := range authInfos { + var pullErr error + klog.V(5).Infof("Pull image %v:%v with user %v", imageName, tag, authInfo.Username) + pullImageReq.Auth = &runtimeapi.AuthConfig{ + Username: authInfo.Username, + Password: authInfo.Password, } - if len(pullErrs) > 0 { - err = utilerrors.NewAggregate(pullErrs) + _, pullErr = c.criImageClient.PullImage(ctx, pullImageReq) + if pullErr == nil { + pipeW.CloseWithError(io.EOF) + return newImagePullStatusReader(pipeR), nil } + klog.Warningf("Failed to pull image %v:%v with user %v, err %v", imageName, tag, authInfo.Username, pullErr) + pullErrs = append(pullErrs, pullErr) + + } + if len(pullErrs) > 0 { + err = utilerrors.NewAggregate(pullErrs) } + } else { + klog.Errorf("Failed to convert to auth info for registry, err %v", err) } // Try the default secret diff --git a/pkg/daemon/criruntime/imageruntime/docker.go b/pkg/daemon/criruntime/imageruntime/docker.go index 27892c4c51..1156881035 100644 --- a/pkg/daemon/criruntime/imageruntime/docker.go +++ b/pkg/daemon/criruntime/imageruntime/docker.go @@ -81,26 +81,26 @@ func (d *dockerImageService) PullImage(ctx context.Context, imageName, tag strin fullName := imageName + ":" + tag var ioReader io.ReadCloser - if len(pullSecrets) > 0 { - var authInfos []daemonutil.AuthInfo - authInfos, err = secret.ConvertToRegistryAuths(pullSecrets, registry) - if err == nil { - var pullErrs []error - for _, authInfo := range authInfos { - var pullErr error - klog.V(5).Infof("Pull image %v:%v with user %v", imageName, tag, authInfo.Username) - ioReader, pullErr = d.client.ImagePull(ctx, fullName, dockertypes.ImagePullOptions{RegistryAuth: authInfo.EncodeToString()}) - if pullErr == nil { - return newImagePullStatusReader(ioReader), nil - } - d.handleRuntimeError(pullErr) - klog.Warningf("Failed to pull image %v:%v with user %v, err %v", imageName, tag, authInfo.Username, pullErr) - pullErrs = append(pullErrs, pullErr) - } - if len(pullErrs) > 0 { - err = utilerrors.NewAggregate(pullErrs) + var authInfos []daemonutil.AuthInfo + authInfos, err = secret.ConvertToRegistryAuths(pullSecrets, registry) + if err == nil { + var pullErrs []error + for _, authInfo := range authInfos { + var pullErr error + klog.V(5).Infof("Pull image %v:%v with user %v", imageName, tag, authInfo.Username) + ioReader, pullErr = d.client.ImagePull(ctx, fullName, dockertypes.ImagePullOptions{RegistryAuth: authInfo.EncodeToString()}) + if pullErr == nil { + return newImagePullStatusReader(ioReader), nil } + d.handleRuntimeError(pullErr) + klog.Warningf("Failed to pull image %v:%v with user %v, err %v", imageName, tag, authInfo.Username, pullErr) + pullErrs = append(pullErrs, pullErr) + } + if len(pullErrs) > 0 { + err = utilerrors.NewAggregate(pullErrs) } + } else { + klog.Errorf("Failed to convert to auth info for registry, err %v", err) } // Try the default secret diff --git a/pkg/daemon/criruntime/imageruntime/fake_plugin/main.go b/pkg/daemon/criruntime/imageruntime/fake_plugin/main.go new file mode 100644 index 0000000000..e19814b3ce --- /dev/null +++ b/pkg/daemon/criruntime/imageruntime/fake_plugin/main.go @@ -0,0 +1,152 @@ +package main + +import ( + "bufio" + "context" + "errors" + "fmt" + "io" + "os" + "strings" + + "github.com/spf13/cobra" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/serializer" + "k8s.io/apimachinery/pkg/runtime/serializer/json" + "k8s.io/component-base/logs" + "k8s.io/klog/v2" + "k8s.io/kubelet/pkg/apis/credentialprovider/install" + v1 "k8s.io/kubelet/pkg/apis/credentialprovider/v1" +) + +var ( + scheme = runtime.NewScheme() + codecs = serializer.NewCodecFactory(scheme) +) + +func init() { + install.Install(scheme) +} + +func getCredentials(ctx context.Context, image string, args []string) (*v1.CredentialProviderResponse, error) { + response := &v1.CredentialProviderResponse{ + CacheKeyType: v1.RegistryPluginCacheKeyType, + Auth: map[string]v1.AuthConfig{ + "registry.plugin.com/test": { + Username: "user", + Password: "password", + }, + }, + } + return response, nil +} + +func runPlugin(ctx context.Context, r io.Reader, w io.Writer, args []string) error { + data, err := io.ReadAll(r) + if err != nil { + return err + } + + _, err = json.DefaultMetaFactory.Interpret(data) + if err != nil { + return err + } + + request, err := decodeRequest(data) + if err != nil { + return err + } + + if request.Image == "" { + return errors.New("image in plugin request was empty") + } + + // Deny all requests except for those where the image URL contains registry.plugin.com + // to test whether kruise could get expected auths if plugin fails to run + if !strings.Contains(request.Image, "registry.plugin.com") { + return errors.New("image in plugin request not supported: " + request.Image) + } + + response, err := getCredentials(ctx, request.Image, args) + if err != nil { + return err + } + + if response == nil { + return errors.New("CredentialProviderResponse from plugin was nil") + } + + encodedResponse, err := encodeResponse(response) + if err != nil { + return err + } + + writer := bufio.NewWriter(w) + defer writer.Flush() + if _, err := writer.Write(encodedResponse); err != nil { + return err + } + + return nil +} + +func decodeRequest(data []byte) (*v1.CredentialProviderRequest, error) { + obj, gvk, err := codecs.UniversalDecoder(v1.SchemeGroupVersion).Decode(data, nil, nil) + if err != nil { + return nil, err + } + + if gvk.Kind != "CredentialProviderRequest" { + return nil, fmt.Errorf("kind was %q, expected CredentialProviderRequest", gvk.Kind) + } + + if gvk.Group != v1.GroupName { + return nil, fmt.Errorf("group was %q, expected %s", gvk.Group, v1.GroupName) + } + + request, ok := obj.(*v1.CredentialProviderRequest) + if !ok { + return nil, fmt.Errorf("unable to convert %T to *CredentialProviderRequest", obj) + } + + return request, nil +} + +func encodeResponse(response *v1.CredentialProviderResponse) ([]byte, error) { + mediaType := "application/json" + info, ok := runtime.SerializerInfoForMediaType(codecs.SupportedMediaTypes(), mediaType) + if !ok { + return nil, fmt.Errorf("unsupported media type %q", mediaType) + } + + encoder := codecs.EncoderForVersion(info.Serializer, v1.SchemeGroupVersion) + data, err := runtime.Encode(encoder, response) + if err != nil { + return nil, fmt.Errorf("failed to encode response: %v", err) + } + + return data, nil +} + +func main() { + logs.InitLogs() + defer logs.FlushLogs() + + if err := newCredentialProviderCommand().Execute(); err != nil { + os.Exit(1) + } +} + +func newCredentialProviderCommand() *cobra.Command { + cmd := &cobra.Command{ + Use: "acr-credential-provider", + Short: "ACR credential provider for kubelet", + Run: func(cmd *cobra.Command, args []string) { + if err := runPlugin(context.TODO(), os.Stdin, os.Stdout, os.Args[1:]); err != nil { + klog.Errorf("Error running credential provider plugin: %v", err) + os.Exit(1) + } + }, + } + return cmd +} diff --git a/pkg/daemon/criruntime/imageruntime/fake_plugin/plugin-config.yaml b/pkg/daemon/criruntime/imageruntime/fake_plugin/plugin-config.yaml new file mode 100644 index 0000000000..d3ff288b21 --- /dev/null +++ b/pkg/daemon/criruntime/imageruntime/fake_plugin/plugin-config.yaml @@ -0,0 +1,9 @@ +apiVersion: kubelet.config.k8s.io/v1 +kind: CredentialProviderConfig +providers: + - name: fake-credential-plugin + matchImages: + - "registry.plugin.com" + - "registry.private.com" + defaultCacheDuration: "12h" + apiVersion: credentialprovider.kubelet.k8s.io/v1 \ No newline at end of file diff --git a/pkg/daemon/criruntime/imageruntime/helpers_test.go b/pkg/daemon/criruntime/imageruntime/helpers_test.go index a4a8039845..d04362c71d 100644 --- a/pkg/daemon/criruntime/imageruntime/helpers_test.go +++ b/pkg/daemon/criruntime/imageruntime/helpers_test.go @@ -20,6 +20,8 @@ import ( "testing" v1 "k8s.io/api/core/v1" + "k8s.io/klog/v2" + "k8s.io/kubernetes/pkg/credentialprovider/plugin" "k8s.io/kubernetes/pkg/util/parsers" "github.com/openkruise/kruise/pkg/util/secret" @@ -103,7 +105,26 @@ func TestMatchRegistryAuths(t *testing.T) { }, ExpectMinValue: 0, }, + { + name: "test credential plugin if matched", + Image: "registry.plugin.com/test/echoserver:v1", + GetSecrets: func() []v1.Secret { + return []v1.Secret{} + }, + ExpectMinValue: 1, + }, + } + pluginBinDir := "fake_plugin" + pluginConfigFile := "fake_plugin/plugin-config.yaml" + // credential plugin is configured for images with "registry.plugin.com" and "registry.private.com", + // however, only images with "registry.plugin.com" will return a fake credential, + // other images will be denied by the plugin and an error will be raised, + // this is to test whether kruise could get expected auths if plugin fails to run + err := plugin.RegisterCredentialProviderPlugins(pluginConfigFile, pluginBinDir) + if err != nil { + klog.Errorf("Failed to register credential provider plugins: %v", err) } + secret.MakeAndSetKeyring() for _, cs := range cases { t.Run(cs.name, func(t *testing.T) { repoToPull, _, _, err := parsers.ParseImageName(cs.Image) diff --git a/pkg/daemon/criruntime/imageruntime/pouch.go b/pkg/daemon/criruntime/imageruntime/pouch.go index 9d9b7ef5e4..f85957cd13 100644 --- a/pkg/daemon/criruntime/imageruntime/pouch.go +++ b/pkg/daemon/criruntime/imageruntime/pouch.go @@ -80,26 +80,26 @@ func (d *pouchImageService) PullImage(ctx context.Context, imageName, tag string registry := daemonutil.ParseRegistry(imageName) var ioReader io.ReadCloser - if len(pullSecrets) > 0 { - var authInfos []daemonutil.AuthInfo - authInfos, err = secret.ConvertToRegistryAuths(pullSecrets, registry) - if err == nil { - var pullErrs []error - for _, authInfo := range authInfos { - var pullErr error - klog.V(5).Infof("Pull image %v:%v with user %v", imageName, tag, authInfo.Username) - ioReader, pullErr = d.client.ImagePull(ctx, imageName, tag, authInfo.EncodeToString()) - if pullErr == nil { - return newImagePullStatusReader(ioReader), nil - } - d.handleRuntimeError(pullErr) - klog.Warningf("Failed to pull image %v:%v with user %v, err %v", imageName, tag, authInfo.Username, pullErr) - pullErrs = append(pullErrs, pullErr) - } - if len(pullErrs) > 0 { - err = utilerrors.NewAggregate(pullErrs) + var authInfos []daemonutil.AuthInfo + authInfos, err = secret.ConvertToRegistryAuths(pullSecrets, registry) + if err == nil { + var pullErrs []error + for _, authInfo := range authInfos { + var pullErr error + klog.V(5).Infof("Pull image %v:%v with user %v", imageName, tag, authInfo.Username) + ioReader, pullErr = d.client.ImagePull(ctx, imageName, tag, authInfo.EncodeToString()) + if pullErr == nil { + return newImagePullStatusReader(ioReader), nil } + d.handleRuntimeError(pullErr) + klog.Warningf("Failed to pull image %v:%v with user %v, err %v", imageName, tag, authInfo.Username, pullErr) + pullErrs = append(pullErrs, pullErr) + } + if len(pullErrs) > 0 { + err = utilerrors.NewAggregate(pullErrs) } + } else { + klog.Errorf("Failed to convert to auth info for registry, err %v", err) } // Try the default secret diff --git a/pkg/util/secret/parse.go b/pkg/util/secret/parse.go index 5ed48b7309..d7334f1584 100644 --- a/pkg/util/secret/parse.go +++ b/pkg/util/secret/parse.go @@ -6,6 +6,7 @@ import ( daemonutil "github.com/openkruise/kruise/pkg/daemon/util" corev1 "k8s.io/api/core/v1" + "k8s.io/klog/v2" "k8s.io/kubernetes/pkg/credentialprovider" credentialprovidersecrets "k8s.io/kubernetes/pkg/credentialprovider/secrets" ) @@ -22,10 +23,19 @@ func AuthInfos(ctx context.Context, imageName, tag string, pullSecrets []corev1. } var ( - keyring = credentialprovider.NewDockerKeyring() + keyring credentialprovider.DockerKeyring ) +// make and set new docker keyring +func MakeAndSetKeyring() { + klog.Infof("make and set new docker keyring") + keyring = credentialprovider.NewDockerKeyring() +} + func ConvertToRegistryAuths(pullSecrets []corev1.Secret, repo string) (infos []daemonutil.AuthInfo, err error) { + if keyring == nil { + MakeAndSetKeyring() + } keyring, err := credentialprovidersecrets.MakeDockerKeyring(pullSecrets, keyring) if err != nil { return nil, err