From 0da73a0d43ff08e33d2201a9d3445025f05c057b Mon Sep 17 00:00:00 2001 From: ppeau Date: Mon, 20 Apr 2026 14:19:26 -0400 Subject: [PATCH 1/3] feat: add imagePullSecrets support for container-based skills Add authentication support for pulling skill images from private registries (Artifactory, ACR, ECR, etc.) by introducing a new imagePullSecrets field under spec.skills. When imagePullSecrets is set, a docker-auth-init init container is prepended that merges all kubernetes.io/dockerconfigjson secrets into a single config.json using jq. The skills-init container then reads that config via the DOCKER_CONFIG env var, which krane picks up automatically when pulling skill images. Closes #1222 Signed-off-by: ppeau --- docker/skills-init/Dockerfile | 2 +- .../config/crd/bases/kagent.dev_agents.yaml | 25 ++ .../crd/bases/kagent.dev_sandboxagents.yaml | 25 ++ go/api/v1alpha2/agent_types.go | 9 + go/api/v1alpha2/zz_generated.deepcopy.go | 5 + .../translator/agent/adk_api_translator.go | 93 +++++++- .../translator/agent/git_skills_test.go | 218 ++++++++++++++++++ .../translator/agent/manifest_builder.go | 3 +- go/core/test/e2e/invoke_api_test.go | 63 +++++ .../templates/kagent.dev_agents.yaml | 25 ++ .../templates/kagent.dev_sandboxagents.yaml | 25 ++ 11 files changed, 486 insertions(+), 7 deletions(-) diff --git a/docker/skills-init/Dockerfile b/docker/skills-init/Dockerfile index e884f34cf..dc89810f7 100644 --- a/docker/skills-init/Dockerfile +++ b/docker/skills-init/Dockerfile @@ -17,7 +17,7 @@ FROM alpine:3.23 ARG PYTHON_UID=1001 ARG PYTHON_GID=1001 -RUN apk upgrade --no-cache && apk add --no-cache git +RUN apk upgrade --no-cache && apk add --no-cache git jq COPY --from=krane-builder /build/krane /usr/local/bin/krane # Run as the same UID/GID as the main agent container (python user) so that diff --git a/go/api/config/crd/bases/kagent.dev_agents.yaml b/go/api/config/crd/bases/kagent.dev_agents.yaml index a91a31b27..3ed715415 100644 --- a/go/api/config/crd/bases/kagent.dev_agents.yaml +++ b/go/api/config/crd/bases/kagent.dev_agents.yaml @@ -13316,6 +13316,31 @@ spec: maxItems: 20 minItems: 1 type: array + imagePullSecrets: + description: |- + ImagePullSecrets is a list of references to secrets in the same namespace to use for + pulling skill images from private registries. Each referenced secret must be of type + kubernetes.io/dockerconfigjson. The credentials from all secrets are merged and made + available to the skills-init container at /.kagent/.docker/config.json; krane will + use them automatically when pulling images. + items: + description: |- + LocalObjectReference contains enough information to let you locate the + referenced object inside the same namespace. + properties: + name: + default: "" + description: |- + Name of the referent. + This field is effectively required, but due to backwards compatibility is + allowed to be empty. Instances of this type with an empty value here are + almost certainly wrong. + More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + type: string + type: object + x-kubernetes-map-type: atomic + maxItems: 20 + type: array initContainer: description: Configuration for the skills-init init container. properties: diff --git a/go/api/config/crd/bases/kagent.dev_sandboxagents.yaml b/go/api/config/crd/bases/kagent.dev_sandboxagents.yaml index 72dc42938..9de4adf06 100644 --- a/go/api/config/crd/bases/kagent.dev_sandboxagents.yaml +++ b/go/api/config/crd/bases/kagent.dev_sandboxagents.yaml @@ -10967,6 +10967,31 @@ spec: maxItems: 20 minItems: 1 type: array + imagePullSecrets: + description: |- + ImagePullSecrets is a list of references to secrets in the same namespace to use for + pulling skill images from private registries. Each referenced secret must be of type + kubernetes.io/dockerconfigjson. The credentials from all secrets are merged and made + available to the skills-init container at /.kagent/.docker/config.json; krane will + use them automatically when pulling images. + items: + description: |- + LocalObjectReference contains enough information to let you locate the + referenced object inside the same namespace. + properties: + name: + default: "" + description: |- + Name of the referent. + This field is effectively required, but due to backwards compatibility is + allowed to be empty. Instances of this type with an empty value here are + almost certainly wrong. + More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + type: string + type: object + x-kubernetes-map-type: atomic + maxItems: 20 + type: array initContainer: description: Configuration for the skills-init init container. properties: diff --git a/go/api/v1alpha2/agent_types.go b/go/api/v1alpha2/agent_types.go index dfe8513d1..c0f3aa532 100644 --- a/go/api/v1alpha2/agent_types.go +++ b/go/api/v1alpha2/agent_types.go @@ -98,6 +98,15 @@ type SkillForAgent struct { // +optional Refs []string `json:"refs,omitempty"` + // ImagePullSecrets is a list of references to secrets in the same namespace to use for + // pulling skill images from private registries. Each referenced secret must be of type + // kubernetes.io/dockerconfigjson. The credentials from all secrets are merged and made + // available to the skills-init container at /.kagent/.docker/config.json; krane will + // use them automatically when pulling images. + // +optional + // +kubebuilder:validation:MaxItems=20 + ImagePullSecrets []corev1.LocalObjectReference `json:"imagePullSecrets,omitempty"` + // Reference to a Secret containing git credentials. // Applied to all gitRefs entries. // The secret should contain a `token` key for HTTPS auth, diff --git a/go/api/v1alpha2/zz_generated.deepcopy.go b/go/api/v1alpha2/zz_generated.deepcopy.go index 47615e2d4..136058bce 100644 --- a/go/api/v1alpha2/zz_generated.deepcopy.go +++ b/go/api/v1alpha2/zz_generated.deepcopy.go @@ -1696,6 +1696,11 @@ func (in *SkillForAgent) DeepCopyInto(out *SkillForAgent) { *out = make([]string, len(*in)) copy(*out, *in) } + if in.ImagePullSecrets != nil { + in, out := &in.ImagePullSecrets, &out.ImagePullSecrets + *out = make([]v1.LocalObjectReference, len(*in)) + copy(*out, *in) + } if in.GitAuthSecretRef != nil { in, out := &in.GitAuthSecretRef, &out.GitAuthSecretRef *out = new(v1.LocalObjectReference) diff --git a/go/core/internal/controller/translator/agent/adk_api_translator.go b/go/core/internal/controller/translator/agent/adk_api_translator.go index b11d030aa..6e2e7211d 100644 --- a/go/core/internal/controller/translator/agent/adk_api_translator.go +++ b/go/core/internal/controller/translator/agent/adk_api_translator.go @@ -1324,6 +1324,9 @@ func prepareSkillsInitData( // buildSkillsInitContainer creates the unified init container and associated volumes // for fetching skills from both Git repositories and OCI registries. // If authSecretRef is non-nil a single Secret volume is created and mounted at /git-auth. +// If imagePullSecrets is non-empty, a docker-auth-init container is prepended that merges +// all kubernetes.io/dockerconfigjson secrets into /.kagent/.docker/config.json; krane +// reads the directory via the DOCKER_CONFIG env var and uses those credentials automatically. func buildSkillsInitContainer( gitRefs []v1alpha2.GitRepo, authSecretRef *corev1.LocalObjectReference, @@ -1332,14 +1335,15 @@ func buildSkillsInitContainer( securityContext *corev1.SecurityContext, env []corev1.EnvVar, resources corev1.ResourceRequirements, -) (container corev1.Container, volumes []corev1.Volume, err error) { + imagePullSecrets []corev1.LocalObjectReference, +) (containers []corev1.Container, volumes []corev1.Volume, err error) { data, err := prepareSkillsInitData(gitRefs, authSecretRef, ociRefs, insecureOCI) if err != nil { - return corev1.Container{}, nil, err + return nil, nil, err } script, err := buildSkillsScript(data) if err != nil { - return corev1.Container{}, nil, err + return nil, nil, err } initSecCtx := securityContext if initSecCtx != nil { @@ -1367,7 +1371,57 @@ func buildSkillsInitContainer( }) } - container = corev1.Container{ + // If imagePullSecrets are specified, build a docker-auth-init container that merges all + // kubernetes.io/dockerconfigjson secrets into a single config.json using jq, then mount + // the result into the skills-init container so krane can authenticate to private registries. + if len(imagePullSecrets) > 0 { + // Shared EmptyDir volume for the merged Docker config. + volumes = append(volumes, corev1.Volume{ + Name: "kagent-docker-config", + VolumeSource: corev1.VolumeSource{ + EmptyDir: &corev1.EmptyDirVolumeSource{}, + }, + }) + + // Mount each imagePullSecret as a read-only directory under /docker-secrets/. + authInitVolumeMounts := []corev1.VolumeMount{ + {Name: "kagent-docker-config", MountPath: "/docker-config-out"}, + } + for _, secret := range imagePullSecrets { + volName := "pull-secret-" + secret.Name + volumes = append(volumes, corev1.Volume{ + Name: volName, + VolumeSource: corev1.VolumeSource{ + Secret: &corev1.SecretVolumeSource{ + SecretName: secret.Name, + }, + }, + }) + authInitVolumeMounts = append(authInitVolumeMounts, corev1.VolumeMount{ + Name: volName, + MountPath: "/docker-secrets/" + secret.Name, + ReadOnly: true, + }) + } + + mergeScript := buildDockerAuthMergeScript(imagePullSecrets) + dockerAuthInitContainer := corev1.Container{ + Name: "docker-auth-init", + Image: DefaultSkillsInitImageConfig.Image(), + Command: []string{"/bin/sh", "-c", mergeScript}, + VolumeMounts: authInitVolumeMounts, + } + containers = append(containers, dockerAuthInitContainer) + + // Mount the merged config into skills-init so krane picks it up via DOCKER_CONFIG. + volumeMounts = append(volumeMounts, corev1.VolumeMount{ + Name: "kagent-docker-config", + MountPath: "/.kagent/.docker", + ReadOnly: true, + }) + } + + skillsInitContainer := corev1.Container{ Name: "skills-init", Image: DefaultSkillsInitImageConfig.Image(), Command: []string{"/bin/sh", "-c", script}, @@ -1377,7 +1431,36 @@ func buildSkillsInitContainer( Resources: resources, } - return container, volumes, nil + // If a merged Docker config is available, point krane to it via DOCKER_CONFIG. + if len(imagePullSecrets) > 0 { + skillsInitContainer.Env = append(skillsInitContainer.Env, corev1.EnvVar{ + Name: "DOCKER_CONFIG", + Value: "/.kagent/.docker", + }) + } + + containers = append(containers, skillsInitContainer) + return containers, volumes, nil +} + +// buildDockerAuthMergeScript generates a shell script that merges the .auths sections from +// all kubernetes.io/dockerconfigjson secrets (mounted under /docker-secrets//) into a +// single Docker config.json at /docker-config-out/config.json using jq. +func buildDockerAuthMergeScript(imagePullSecrets []corev1.LocalObjectReference) string { + var sb strings.Builder + sb.WriteString(`set -e +mkdir -p /docker-config-out +merged='{"auths":{}}' +`) + for _, secret := range imagePullSecrets { + sb.WriteString(`if [ -f /docker-secrets/` + secret.Name + `/.dockerconfigjson ]; then + merged="$(printf '%s\n%s\n' "$merged" "$(cat /docker-secrets/` + secret.Name + `/.dockerconfigjson)" | jq -s '.[0].auths * .[1].auths | {"auths": .}')" +fi +`) + } + sb.WriteString(`printf '%s' "$merged" > /docker-config-out/config.json +`) + return sb.String() } func (a *adkApiTranslator) runPlugins(ctx context.Context, agent v1alpha2.AgentObject, outputs *AgentOutputs) error { diff --git a/go/core/internal/controller/translator/agent/git_skills_test.go b/go/core/internal/controller/translator/agent/git_skills_test.go index b5541140d..53b0d32ca 100644 --- a/go/core/internal/controller/translator/agent/git_skills_test.go +++ b/go/core/internal/controller/translator/agent/git_skills_test.go @@ -428,6 +428,224 @@ func Test_AdkApiTranslator_Skills(t *testing.T) { } } +func Test_AdkApiTranslator_SkillsImagePullSecrets(t *testing.T) { + scheme := schemev1.Scheme + require.NoError(t, v1alpha2.AddToScheme(scheme)) + + namespace := "default" + modelName := "test-model" + + modelConfig := &v1alpha2.ModelConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: modelName, + Namespace: namespace, + }, + Spec: v1alpha2.ModelConfigSpec{ + Model: "gpt-4", + Provider: v1alpha2.ModelProviderOpenAI, + }, + } + + defaultModel := types.NamespacedName{ + Namespace: namespace, + Name: modelName, + } + + tests := []struct { + name string + agent *v1alpha2.Agent + wantDockerAuthInit bool + wantInitCount int + }{ + { + name: "OCI skills without imagePullSecrets - single init container", + agent: &v1alpha2.Agent{ + ObjectMeta: metav1.ObjectMeta{Name: "agent-no-pull-secret", Namespace: namespace}, + Spec: v1alpha2.AgentSpec{ + Type: v1alpha2.AgentType_Declarative, + Declarative: &v1alpha2.DeclarativeAgentSpec{ + SystemMessage: "test", + ModelConfig: modelName, + }, + Skills: &v1alpha2.SkillForAgent{ + Refs: []string{"ghcr.io/org/skill:v1"}, + }, + }, + }, + wantDockerAuthInit: false, + wantInitCount: 1, + }, + { + name: "OCI skills with single imagePullSecret - two init containers", + agent: &v1alpha2.Agent{ + ObjectMeta: metav1.ObjectMeta{Name: "agent-one-pull-secret", Namespace: namespace}, + Spec: v1alpha2.AgentSpec{ + Type: v1alpha2.AgentType_Declarative, + Declarative: &v1alpha2.DeclarativeAgentSpec{ + SystemMessage: "test", + ModelConfig: modelName, + }, + Skills: &v1alpha2.SkillForAgent{ + Refs: []string{"docker.artifactory.example.com/org/skill:v1"}, + ImagePullSecrets: []corev1.LocalObjectReference{{Name: "registry-credentials"}}, + }, + }, + }, + wantDockerAuthInit: true, + wantInitCount: 2, + }, + { + name: "OCI skills with multiple imagePullSecrets - two init containers merging all auths", + agent: &v1alpha2.Agent{ + ObjectMeta: metav1.ObjectMeta{Name: "agent-multi-pull-secrets", Namespace: namespace}, + Spec: v1alpha2.AgentSpec{ + Type: v1alpha2.AgentType_Declarative, + Declarative: &v1alpha2.DeclarativeAgentSpec{ + SystemMessage: "test", + ModelConfig: modelName, + }, + Skills: &v1alpha2.SkillForAgent{ + Refs: []string{ + "docker.artifactory.example.com/org/skill-a:v1", + "acr.azurecr.io/org/skill-b:v2", + }, + ImagePullSecrets: []corev1.LocalObjectReference{ + {Name: "artifactory-creds"}, + {Name: "acr-creds"}, + }, + }, + }, + }, + wantDockerAuthInit: true, + wantInitCount: 2, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + kubeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(modelConfig, tt.agent). + Build() + + trans := translator.NewAdkApiTranslator(kubeClient, defaultModel, nil, "", nil) + + outputs, err := translator.TranslateAgent(context.Background(), trans, tt.agent) + require.NoError(t, err) + require.NotNil(t, outputs) + + var deployment *appsv1.Deployment + for _, obj := range outputs.Manifest { + if d, ok := obj.(*appsv1.Deployment); ok { + deployment = d + } + } + require.NotNil(t, deployment, "Deployment should be created") + + initContainers := deployment.Spec.Template.Spec.InitContainers + assert.Len(t, initContainers, tt.wantInitCount, "unexpected number of init containers") + + // Find the skills-init container + var skillsInitContainer *corev1.Container + for i := range initContainers { + if initContainers[i].Name == "skills-init" { + skillsInitContainer = &initContainers[i] + } + } + require.NotNil(t, skillsInitContainer, "skills-init container should always exist") + + if tt.wantDockerAuthInit { + // Verify docker-auth-init container exists and is the first init container + require.True(t, len(initContainers) >= 2, "should have at least 2 init containers") + assert.Equal(t, "docker-auth-init", initContainers[0].Name, "docker-auth-init should be first") + + dockerAuthInit := &initContainers[0] + + // Verify merge script uses jq and writes to the correct path + require.Len(t, dockerAuthInit.Command, 3) + mergeScript := dockerAuthInit.Command[2] + assert.Contains(t, mergeScript, "jq") + assert.Contains(t, mergeScript, ".dockerconfigjson") + assert.Contains(t, mergeScript, "/docker-config-out/config.json") + + // Verify docker-config-out volume mount exists on docker-auth-init + hasConfigOut := false + for _, vm := range dockerAuthInit.VolumeMounts { + if vm.Name == "kagent-docker-config" && vm.MountPath == "/docker-config-out" { + hasConfigOut = true + } + } + assert.True(t, hasConfigOut, "docker-auth-init should mount kagent-docker-config at /docker-config-out") + + // Verify pull secret volumes and mounts are present on docker-auth-init + require.NotNil(t, tt.agent.Spec.Skills) + for _, ps := range tt.agent.Spec.Skills.ImagePullSecrets { + volName := "pull-secret-" + ps.Name + + // Volume on deployment + hasPullSecretVol := false + for _, v := range deployment.Spec.Template.Spec.Volumes { + if v.Name == volName && v.Secret != nil && v.Secret.SecretName == ps.Name { + hasPullSecretVol = true + } + } + assert.True(t, hasPullSecretVol, "pull-secret volume %q should exist", volName) + + // Mount on docker-auth-init + hasPullSecretMount := false + for _, vm := range dockerAuthInit.VolumeMounts { + if vm.Name == volName && vm.MountPath == "/docker-secrets/"+ps.Name && vm.ReadOnly { + hasPullSecretMount = true + } + } + assert.True(t, hasPullSecretMount, "docker-auth-init should mount pull-secret %q", volName) + + // Merge script references this secret + assert.Contains(t, mergeScript, "/docker-secrets/"+ps.Name+"/.dockerconfigjson") + } + + // Verify shared EmptyDir volume exists + hasDockerConfigVol := false + for _, v := range deployment.Spec.Template.Spec.Volumes { + if v.Name == "kagent-docker-config" { + hasDockerConfigVol = true + assert.NotNil(t, v.EmptyDir, "kagent-docker-config should be an EmptyDir volume") + } + } + assert.True(t, hasDockerConfigVol, "kagent-docker-config EmptyDir volume should exist") + + // Verify skills-init mounts the docker config and has DOCKER_CONFIG env + hasDockerMount := false + for _, vm := range skillsInitContainer.VolumeMounts { + if vm.Name == "kagent-docker-config" && vm.MountPath == "/.kagent/.docker" && vm.ReadOnly { + hasDockerMount = true + } + } + assert.True(t, hasDockerMount, "skills-init should mount kagent-docker-config at /.kagent/.docker") + + hasDockerConfigEnv := false + for _, e := range skillsInitContainer.Env { + if e.Name == "DOCKER_CONFIG" && e.Value == "/.kagent/.docker" { + hasDockerConfigEnv = true + } + } + assert.True(t, hasDockerConfigEnv, "skills-init should have DOCKER_CONFIG env var pointing to /.kagent/.docker") + } else { + // No imagePullSecrets: no docker-auth-init, no DOCKER_CONFIG env, no docker config volumes + for _, c := range initContainers { + assert.NotEqual(t, "docker-auth-init", c.Name, "docker-auth-init should not exist without imagePullSecrets") + } + for _, e := range skillsInitContainer.Env { + assert.NotEqual(t, "DOCKER_CONFIG", e.Name, "DOCKER_CONFIG env should not be set without imagePullSecrets") + } + for _, v := range deployment.Spec.Template.Spec.Volumes { + assert.NotEqual(t, "kagent-docker-config", v.Name, "kagent-docker-config volume should not exist") + } + } + }) + } +} + func Test_AdkApiTranslator_SkillsConfigurableImage(t *testing.T) { scheme := schemev1.Scheme require.NoError(t, v1alpha2.AddToScheme(scheme)) diff --git a/go/core/internal/controller/translator/agent/manifest_builder.go b/go/core/internal/controller/translator/agent/manifest_builder.go index 5a4e543fa..dcbaee9c4 100644 --- a/go/core/internal/controller/translator/agent/manifest_builder.go +++ b/go/core/internal/controller/translator/agent/manifest_builder.go @@ -386,13 +386,14 @@ func buildSkillsRuntime( manifestCtx.deployment.SecurityContext, initEnv, getDefaultResources(initResources), + spec.Skills.ImagePullSecrets, ) if err != nil { return nil, fmt.Errorf("failed to build skills init container: %w", err) } *volumes = append(*volumes, skillsVolumes...) - return []corev1.Container{container}, nil + return container, nil } func projectedTokenVolume() corev1.Volume { diff --git a/go/core/test/e2e/invoke_api_test.go b/go/core/test/e2e/invoke_api_test.go index b8de767d7..b463f1da9 100644 --- a/go/core/test/e2e/invoke_api_test.go +++ b/go/core/test/e2e/invoke_api_test.go @@ -13,6 +13,7 @@ import ( "testing" "time" + appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" k8s_runtime "k8s.io/apimachinery/pkg/runtime" @@ -83,6 +84,8 @@ func setupK8sClient(t *testing.T, includeV1Alpha1 bool) client.Client { } err = corev1.AddToScheme(scheme) require.NoError(t, err) + err = appsv1.AddToScheme(scheme) + require.NoError(t, err) cli, err := client.New(cfg, client.Options{ Scheme: scheme, @@ -1146,6 +1149,66 @@ func TestE2EInvokeSkillInAgent(t *testing.T) { runSyncTest(t, a2aClient, "make me a kebab", "Pick it up from around the corner", nil) } +func TestE2ESkillImagePullSecrets(t *testing.T) { + // Setup mock server + baseURL, stopServer := setupMockServer(t, "mocks/invoke_skill.json") + defer stopServer() + + // Setup Kubernetes client + cli := setupK8sClient(t, false) + + // Create a dummy dockerconfigjson secret. + // The kind-registry is unauthenticated, so credentials don't matter — + // we're testing that the controller wires up the docker-auth-init container. + dockerConfigJSON := `{"auths":{"kind-registry:5000":{"username":"user","password":"pass","auth":"dXNlcjpwYXNz"}}}` + pullSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "test-pull-secret-", + Namespace: "kagent", + }, + Type: corev1.SecretTypeDockerConfigJson, + Data: map[string][]byte{ + corev1.DockerConfigJsonKey: []byte(dockerConfigJSON), + }, + } + require.NoError(t, cli.Create(t.Context(), pullSecret)) + cleanup(t, cli, pullSecret) + + // Setup model config and agent with imagePullSecrets + modelCfg := setupModelConfig(t, cli, baseURL) + agent := setupAgentWithOptions(t, cli, modelCfg.Name, nil, AgentOptions{ + Skills: &v1alpha2.SkillForAgent{ + InsecureSkipVerify: true, + Refs: []string{"kind-registry:5000/kebab-maker:latest"}, + ImagePullSecrets: []corev1.LocalObjectReference{ + {Name: pullSecret.Name}, + }, + }, + }) + + // Verify the Deployment has the docker-auth-init init container + deployment := &appsv1.Deployment{} + require.NoError(t, cli.Get(t.Context(), client.ObjectKey{Name: agent.Name, Namespace: agent.Namespace}, deployment)) + initContainers := deployment.Spec.Template.Spec.InitContainers + require.Len(t, initContainers, 2, "expected docker-auth-init + skills-init init containers") + require.Equal(t, "docker-auth-init", initContainers[0].Name) + require.Equal(t, "skills-init", initContainers[1].Name) + + // Verify docker-auth-init mounts all pull secrets + var foundSecretMount bool + for _, vol := range initContainers[0].VolumeMounts { + if strings.Contains(vol.Name, "pull-secret") { + foundSecretMount = true + break + } + } + require.True(t, foundSecretMount, "docker-auth-init should mount the pull secret volume") + + // Verify the agent works end-to-end with the skill + a2aClient := setupA2AClient(t, agent) + runSyncTest(t, a2aClient, "make me a kebab", "Pick it up from around the corner", nil) +} + func TestE2EDeclarativeAgentNetworkAllowlistWithSkills(t *testing.T) { runDeclarativeAgentNetworkAllowlistWithSkills(t, "python", nil) } diff --git a/helm/kagent-crds/templates/kagent.dev_agents.yaml b/helm/kagent-crds/templates/kagent.dev_agents.yaml index a91a31b27..3ed715415 100644 --- a/helm/kagent-crds/templates/kagent.dev_agents.yaml +++ b/helm/kagent-crds/templates/kagent.dev_agents.yaml @@ -13316,6 +13316,31 @@ spec: maxItems: 20 minItems: 1 type: array + imagePullSecrets: + description: |- + ImagePullSecrets is a list of references to secrets in the same namespace to use for + pulling skill images from private registries. Each referenced secret must be of type + kubernetes.io/dockerconfigjson. The credentials from all secrets are merged and made + available to the skills-init container at /.kagent/.docker/config.json; krane will + use them automatically when pulling images. + items: + description: |- + LocalObjectReference contains enough information to let you locate the + referenced object inside the same namespace. + properties: + name: + default: "" + description: |- + Name of the referent. + This field is effectively required, but due to backwards compatibility is + allowed to be empty. Instances of this type with an empty value here are + almost certainly wrong. + More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + type: string + type: object + x-kubernetes-map-type: atomic + maxItems: 20 + type: array initContainer: description: Configuration for the skills-init init container. properties: diff --git a/helm/kagent-crds/templates/kagent.dev_sandboxagents.yaml b/helm/kagent-crds/templates/kagent.dev_sandboxagents.yaml index 72dc42938..9de4adf06 100644 --- a/helm/kagent-crds/templates/kagent.dev_sandboxagents.yaml +++ b/helm/kagent-crds/templates/kagent.dev_sandboxagents.yaml @@ -10967,6 +10967,31 @@ spec: maxItems: 20 minItems: 1 type: array + imagePullSecrets: + description: |- + ImagePullSecrets is a list of references to secrets in the same namespace to use for + pulling skill images from private registries. Each referenced secret must be of type + kubernetes.io/dockerconfigjson. The credentials from all secrets are merged and made + available to the skills-init container at /.kagent/.docker/config.json; krane will + use them automatically when pulling images. + items: + description: |- + LocalObjectReference contains enough information to let you locate the + referenced object inside the same namespace. + properties: + name: + default: "" + description: |- + Name of the referent. + This field is effectively required, but due to backwards compatibility is + allowed to be empty. Instances of this type with an empty value here are + almost certainly wrong. + More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + type: string + type: object + x-kubernetes-map-type: atomic + maxItems: 20 + type: array initContainer: description: Configuration for the skills-init init container. properties: From 0da774bff239c9f2638a15c0d2e92eda5134eaaf Mon Sep 17 00:00:00 2001 From: ppeau Date: Mon, 4 May 2026 11:48:33 -0400 Subject: [PATCH 2/3] refactor: move docker-auth-init script to tmpl file Signed-off-by: ppeau --- .../translator/agent/adk_api_translator.go | 47 +++++++++++-------- .../translator/agent/docker-auth-init.sh.tmpl | 9 ++++ 2 files changed, 37 insertions(+), 19 deletions(-) create mode 100644 go/core/internal/controller/translator/agent/docker-auth-init.sh.tmpl diff --git a/go/core/internal/controller/translator/agent/adk_api_translator.go b/go/core/internal/controller/translator/agent/adk_api_translator.go index 6e2e7211d..31d515a12 100644 --- a/go/core/internal/controller/translator/agent/adk_api_translator.go +++ b/go/core/internal/controller/translator/agent/adk_api_translator.go @@ -1214,6 +1214,17 @@ var skillsInitScriptTmpl string // skillsScriptTemplate is the shell script template for fetching skills from Git and OCI. var skillsScriptTemplate = template.Must(template.New("skills-init").Parse(skillsInitScriptTmpl)) +// dockerAuthInitData holds the secret names for the docker-auth-init script template. +type dockerAuthInitData struct { + Secrets []string +} + +//go:embed docker-auth-init.sh.tmpl +var dockerAuthInitScriptTmpl string + +// dockerAuthInitTemplate is the shell script template for merging Docker auth credentials. +var dockerAuthInitTemplate = template.Must(template.New("docker-auth-init").Parse(dockerAuthInitScriptTmpl)) + // buildSkillsScript renders the unified skills-init shell script. func buildSkillsScript(data skillsInitData) (string, error) { var buf bytes.Buffer @@ -1404,7 +1415,10 @@ func buildSkillsInitContainer( }) } - mergeScript := buildDockerAuthMergeScript(imagePullSecrets) + mergeScript, err := buildDockerAuthMergeScript(imagePullSecrets) + if err != nil { + return nil, nil, err + } dockerAuthInitContainer := corev1.Container{ Name: "docker-auth-init", Image: DefaultSkillsInitImageConfig.Image(), @@ -1443,24 +1457,19 @@ func buildSkillsInitContainer( return containers, volumes, nil } -// buildDockerAuthMergeScript generates a shell script that merges the .auths sections from -// all kubernetes.io/dockerconfigjson secrets (mounted under /docker-secrets//) into a -// single Docker config.json at /docker-config-out/config.json using jq. -func buildDockerAuthMergeScript(imagePullSecrets []corev1.LocalObjectReference) string { - var sb strings.Builder - sb.WriteString(`set -e -mkdir -p /docker-config-out -merged='{"auths":{}}' -`) - for _, secret := range imagePullSecrets { - sb.WriteString(`if [ -f /docker-secrets/` + secret.Name + `/.dockerconfigjson ]; then - merged="$(printf '%s\n%s\n' "$merged" "$(cat /docker-secrets/` + secret.Name + `/.dockerconfigjson)" | jq -s '.[0].auths * .[1].auths | {"auths": .}')" -fi -`) - } - sb.WriteString(`printf '%s' "$merged" > /docker-config-out/config.json -`) - return sb.String() +// buildDockerAuthMergeScript renders the docker-auth-init shell script from the template. +// It merges the .auths sections from all kubernetes.io/dockerconfigjson secrets +// (mounted under /docker-secrets//) into a single config.json using jq. +func buildDockerAuthMergeScript(imagePullSecrets []corev1.LocalObjectReference) (string, error) { + names := make([]string, len(imagePullSecrets)) + for i, s := range imagePullSecrets { + names[i] = s.Name + } + var buf bytes.Buffer + if err := dockerAuthInitTemplate.Execute(&buf, dockerAuthInitData{Secrets: names}); err != nil { + return "", fmt.Errorf("failed to render docker-auth-init script: %w", err) + } + return buf.String(), nil } func (a *adkApiTranslator) runPlugins(ctx context.Context, agent v1alpha2.AgentObject, outputs *AgentOutputs) error { diff --git a/go/core/internal/controller/translator/agent/docker-auth-init.sh.tmpl b/go/core/internal/controller/translator/agent/docker-auth-init.sh.tmpl new file mode 100644 index 000000000..505118d77 --- /dev/null +++ b/go/core/internal/controller/translator/agent/docker-auth-init.sh.tmpl @@ -0,0 +1,9 @@ +set -e +mkdir -p /docker-config-out +merged='{"auths":{}}' +{{- range .Secrets }} +if [ -f /docker-secrets/{{ . }}/.dockerconfigjson ]; then + merged="$(printf '%s\n%s\n' "$merged" "$(cat /docker-secrets/{{ . }}/.dockerconfigjson)" | jq -s '.[0].auths * .[1].auths | {"auths": .}')" +fi +{{- end }} +printf '%s' "$merged" > /docker-config-out/config.json From 1d6b9187dda365b818d6c331137ac5a76957e1c9 Mon Sep 17 00:00:00 2001 From: ppeau Date: Thu, 7 May 2026 19:59:41 -0400 Subject: [PATCH 3/3] refactor: consolidate docker-auth-init logic into skills-init container Previously, when imagePullSecrets were specified, the controller created two init containers: docker-auth-init (to merge dockerconfigjson secrets into a shared EmptyDir volume) and skills-init (to pull OCI/git skills). This commit eliminates the separate docker-auth-init container by embedding the credential merge logic directly into the skills-init shell script template. Each imagePullSecret is now mounted directly on skills-init under /docker-secrets/; the script merges them with jq into /tmp/kagent-docker-config/config.json and exports DOCKER_CONFIG before invoking krane. Changes: - Remove docker-auth-init.sh.tmpl and all associated Go code - Add ImagePullSecrets []string field to skillsInitData - Render credential merge block at top of skills-init.sh.tmpl - Mount pull-secret volumes directly on skills-init container - Update unit tests: assert exactly one init container, no docker-auth-init, no kagent-docker-config EmptyDir volume - Update E2E test: verify single init container and script content Signed-off-by: ppeau --- .../translator/agent/adk_api_translator.go | 119 ++++------------ .../translator/agent/docker-auth-init.sh.tmpl | 9 -- .../translator/agent/git_skills_test.go | 131 ++++++------------ .../translator/agent/skills-init.sh.tmpl | 11 ++ .../translator/agent/skills_unit_test.go | 14 +- go/core/test/e2e/invoke_api_test.go | 24 ++-- 6 files changed, 107 insertions(+), 201 deletions(-) delete mode 100644 go/core/internal/controller/translator/agent/docker-auth-init.sh.tmpl diff --git a/go/core/internal/controller/translator/agent/adk_api_translator.go b/go/core/internal/controller/translator/agent/adk_api_translator.go index 31d515a12..0ee9528f8 100644 --- a/go/core/internal/controller/translator/agent/adk_api_translator.go +++ b/go/core/internal/controller/translator/agent/adk_api_translator.go @@ -1180,11 +1180,12 @@ func validateSubPath(p string) error { // skillsInitData holds the template data for the unified skills-init script. type skillsInitData struct { - AuthMountPath string // "/git-auth" or "" (for git auth) - GitRefs []gitRefData // git repos to clone - OCIRefs []ociRefData // OCI images to pull - InsecureOCI bool // --insecure flag for krane - SSHHosts []sshHostData // extra hosts to add to known_hosts via ssh-keyscan + AuthMountPath string // "/git-auth" or "" (for git auth) + GitRefs []gitRefData // git repos to clone + OCIRefs []ociRefData // OCI images to pull + InsecureOCI bool // --insecure flag for krane + SSHHosts []sshHostData // extra hosts to add to known_hosts via ssh-keyscan + ImagePullSecrets []string // secret names whose .dockerconfigjson are merged by the script } // sshHostData holds the host and optional port for an SSH known_hosts entry. @@ -1214,17 +1215,6 @@ var skillsInitScriptTmpl string // skillsScriptTemplate is the shell script template for fetching skills from Git and OCI. var skillsScriptTemplate = template.Must(template.New("skills-init").Parse(skillsInitScriptTmpl)) -// dockerAuthInitData holds the secret names for the docker-auth-init script template. -type dockerAuthInitData struct { - Secrets []string -} - -//go:embed docker-auth-init.sh.tmpl -var dockerAuthInitScriptTmpl string - -// dockerAuthInitTemplate is the shell script template for merging Docker auth credentials. -var dockerAuthInitTemplate = template.Must(template.New("docker-auth-init").Parse(dockerAuthInitScriptTmpl)) - // buildSkillsScript renders the unified skills-init shell script. func buildSkillsScript(data skillsInitData) (string, error) { var buf bytes.Buffer @@ -1258,9 +1248,11 @@ func prepareSkillsInitData( authSecretRef *corev1.LocalObjectReference, ociRefs []string, insecureOCI bool, + imagePullSecrets []string, ) (skillsInitData, error) { data := skillsInitData{ - InsecureOCI: insecureOCI, + InsecureOCI: insecureOCI, + ImagePullSecrets: imagePullSecrets, } if authSecretRef != nil { @@ -1335,9 +1327,9 @@ func prepareSkillsInitData( // buildSkillsInitContainer creates the unified init container and associated volumes // for fetching skills from both Git repositories and OCI registries. // If authSecretRef is non-nil a single Secret volume is created and mounted at /git-auth. -// If imagePullSecrets is non-empty, a docker-auth-init container is prepended that merges -// all kubernetes.io/dockerconfigjson secrets into /.kagent/.docker/config.json; krane -// reads the directory via the DOCKER_CONFIG env var and uses those credentials automatically. +// If imagePullSecrets is non-empty, each kubernetes.io/dockerconfigjson secret is mounted +// under /docker-secrets/ and the script merges them into a single config.json in /tmp; +// krane reads the credentials via the DOCKER_CONFIG env var exported by the script. func buildSkillsInitContainer( gitRefs []v1alpha2.GitRepo, authSecretRef *corev1.LocalObjectReference, @@ -1348,7 +1340,13 @@ func buildSkillsInitContainer( resources corev1.ResourceRequirements, imagePullSecrets []corev1.LocalObjectReference, ) (containers []corev1.Container, volumes []corev1.Volume, err error) { - data, err := prepareSkillsInitData(gitRefs, authSecretRef, ociRefs, insecureOCI) + // Collect secret names for the script template. + pullSecretNames := make([]string, len(imagePullSecrets)) + for i, s := range imagePullSecrets { + pullSecretNames[i] = s.Name + } + + data, err := prepareSkillsInitData(gitRefs, authSecretRef, ociRefs, insecureOCI, pullSecretNames) if err != nil { return nil, nil, err } @@ -1365,7 +1363,7 @@ func buildSkillsInitContainer( {Name: "kagent-skills", MountPath: "/skills"}, } - // Mount single auth secret if provided + // Mount single auth secret if provided. if authSecretRef != nil { volumes = append(volumes, corev1.Volume{ Name: "git-auth", @@ -1382,55 +1380,21 @@ func buildSkillsInitContainer( }) } - // If imagePullSecrets are specified, build a docker-auth-init container that merges all - // kubernetes.io/dockerconfigjson secrets into a single config.json using jq, then mount - // the result into the skills-init container so krane can authenticate to private registries. - if len(imagePullSecrets) > 0 { - // Shared EmptyDir volume for the merged Docker config. + // Mount each imagePullSecret directly into skills-init under /docker-secrets/. + // The script merges them into /tmp/kagent-docker-config/config.json and exports DOCKER_CONFIG. + for _, secret := range imagePullSecrets { + volName := "pull-secret-" + secret.Name volumes = append(volumes, corev1.Volume{ - Name: "kagent-docker-config", + Name: volName, VolumeSource: corev1.VolumeSource{ - EmptyDir: &corev1.EmptyDirVolumeSource{}, + Secret: &corev1.SecretVolumeSource{ + SecretName: secret.Name, + }, }, }) - - // Mount each imagePullSecret as a read-only directory under /docker-secrets/. - authInitVolumeMounts := []corev1.VolumeMount{ - {Name: "kagent-docker-config", MountPath: "/docker-config-out"}, - } - for _, secret := range imagePullSecrets { - volName := "pull-secret-" + secret.Name - volumes = append(volumes, corev1.Volume{ - Name: volName, - VolumeSource: corev1.VolumeSource{ - Secret: &corev1.SecretVolumeSource{ - SecretName: secret.Name, - }, - }, - }) - authInitVolumeMounts = append(authInitVolumeMounts, corev1.VolumeMount{ - Name: volName, - MountPath: "/docker-secrets/" + secret.Name, - ReadOnly: true, - }) - } - - mergeScript, err := buildDockerAuthMergeScript(imagePullSecrets) - if err != nil { - return nil, nil, err - } - dockerAuthInitContainer := corev1.Container{ - Name: "docker-auth-init", - Image: DefaultSkillsInitImageConfig.Image(), - Command: []string{"/bin/sh", "-c", mergeScript}, - VolumeMounts: authInitVolumeMounts, - } - containers = append(containers, dockerAuthInitContainer) - - // Mount the merged config into skills-init so krane picks it up via DOCKER_CONFIG. volumeMounts = append(volumeMounts, corev1.VolumeMount{ - Name: "kagent-docker-config", - MountPath: "/.kagent/.docker", + Name: volName, + MountPath: "/docker-secrets/" + secret.Name, ReadOnly: true, }) } @@ -1445,33 +1409,10 @@ func buildSkillsInitContainer( Resources: resources, } - // If a merged Docker config is available, point krane to it via DOCKER_CONFIG. - if len(imagePullSecrets) > 0 { - skillsInitContainer.Env = append(skillsInitContainer.Env, corev1.EnvVar{ - Name: "DOCKER_CONFIG", - Value: "/.kagent/.docker", - }) - } - containers = append(containers, skillsInitContainer) return containers, volumes, nil } -// buildDockerAuthMergeScript renders the docker-auth-init shell script from the template. -// It merges the .auths sections from all kubernetes.io/dockerconfigjson secrets -// (mounted under /docker-secrets//) into a single config.json using jq. -func buildDockerAuthMergeScript(imagePullSecrets []corev1.LocalObjectReference) (string, error) { - names := make([]string, len(imagePullSecrets)) - for i, s := range imagePullSecrets { - names[i] = s.Name - } - var buf bytes.Buffer - if err := dockerAuthInitTemplate.Execute(&buf, dockerAuthInitData{Secrets: names}); err != nil { - return "", fmt.Errorf("failed to render docker-auth-init script: %w", err) - } - return buf.String(), nil -} - func (a *adkApiTranslator) runPlugins(ctx context.Context, agent v1alpha2.AgentObject, outputs *AgentOutputs) error { var errs error for _, plugin := range a.plugins { diff --git a/go/core/internal/controller/translator/agent/docker-auth-init.sh.tmpl b/go/core/internal/controller/translator/agent/docker-auth-init.sh.tmpl deleted file mode 100644 index 505118d77..000000000 --- a/go/core/internal/controller/translator/agent/docker-auth-init.sh.tmpl +++ /dev/null @@ -1,9 +0,0 @@ -set -e -mkdir -p /docker-config-out -merged='{"auths":{}}' -{{- range .Secrets }} -if [ -f /docker-secrets/{{ . }}/.dockerconfigjson ]; then - merged="$(printf '%s\n%s\n' "$merged" "$(cat /docker-secrets/{{ . }}/.dockerconfigjson)" | jq -s '.[0].auths * .[1].auths | {"auths": .}')" -fi -{{- end }} -printf '%s' "$merged" > /docker-config-out/config.json diff --git a/go/core/internal/controller/translator/agent/git_skills_test.go b/go/core/internal/controller/translator/agent/git_skills_test.go index 53b0d32ca..a1c49dfe7 100644 --- a/go/core/internal/controller/translator/agent/git_skills_test.go +++ b/go/core/internal/controller/translator/agent/git_skills_test.go @@ -452,13 +452,12 @@ func Test_AdkApiTranslator_SkillsImagePullSecrets(t *testing.T) { } tests := []struct { - name string - agent *v1alpha2.Agent - wantDockerAuthInit bool - wantInitCount int + name string + agent *v1alpha2.Agent + wantImagePullSecret bool }{ { - name: "OCI skills without imagePullSecrets - single init container", + name: "OCI skills without imagePullSecrets - single init container, no credential merge", agent: &v1alpha2.Agent{ ObjectMeta: metav1.ObjectMeta{Name: "agent-no-pull-secret", Namespace: namespace}, Spec: v1alpha2.AgentSpec{ @@ -472,11 +471,10 @@ func Test_AdkApiTranslator_SkillsImagePullSecrets(t *testing.T) { }, }, }, - wantDockerAuthInit: false, - wantInitCount: 1, + wantImagePullSecret: false, }, { - name: "OCI skills with single imagePullSecret - two init containers", + name: "OCI skills with single imagePullSecret - credential merge in skills-init script", agent: &v1alpha2.Agent{ ObjectMeta: metav1.ObjectMeta{Name: "agent-one-pull-secret", Namespace: namespace}, Spec: v1alpha2.AgentSpec{ @@ -491,11 +489,10 @@ func Test_AdkApiTranslator_SkillsImagePullSecrets(t *testing.T) { }, }, }, - wantDockerAuthInit: true, - wantInitCount: 2, + wantImagePullSecret: true, }, { - name: "OCI skills with multiple imagePullSecrets - two init containers merging all auths", + name: "OCI skills with multiple imagePullSecrets - all auths merged in skills-init script", agent: &v1alpha2.Agent{ ObjectMeta: metav1.ObjectMeta{Name: "agent-multi-pull-secrets", Namespace: namespace}, Spec: v1alpha2.AgentSpec{ @@ -516,8 +513,7 @@ func Test_AdkApiTranslator_SkillsImagePullSecrets(t *testing.T) { }, }, }, - wantDockerAuthInit: true, - wantInitCount: 2, + wantImagePullSecret: true, }, } @@ -543,103 +539,64 @@ func Test_AdkApiTranslator_SkillsImagePullSecrets(t *testing.T) { require.NotNil(t, deployment, "Deployment should be created") initContainers := deployment.Spec.Template.Spec.InitContainers - assert.Len(t, initContainers, tt.wantInitCount, "unexpected number of init containers") - // Find the skills-init container - var skillsInitContainer *corev1.Container - for i := range initContainers { - if initContainers[i].Name == "skills-init" { - skillsInitContainer = &initContainers[i] - } + // Always exactly one init container regardless of imagePullSecrets. + assert.Len(t, initContainers, 1, "should always have exactly one init container (skills-init)") + require.Equal(t, "skills-init", initContainers[0].Name, "the single init container must be skills-init") + + skillsInitContainer := &initContainers[0] + require.Len(t, skillsInitContainer.Command, 3) + script := skillsInitContainer.Command[2] + + // No docker-auth-init container should ever exist. + for _, c := range initContainers { + assert.NotEqual(t, "docker-auth-init", c.Name, "docker-auth-init container must not exist") } - require.NotNil(t, skillsInitContainer, "skills-init container should always exist") - - if tt.wantDockerAuthInit { - // Verify docker-auth-init container exists and is the first init container - require.True(t, len(initContainers) >= 2, "should have at least 2 init containers") - assert.Equal(t, "docker-auth-init", initContainers[0].Name, "docker-auth-init should be first") - - dockerAuthInit := &initContainers[0] - - // Verify merge script uses jq and writes to the correct path - require.Len(t, dockerAuthInit.Command, 3) - mergeScript := dockerAuthInit.Command[2] - assert.Contains(t, mergeScript, "jq") - assert.Contains(t, mergeScript, ".dockerconfigjson") - assert.Contains(t, mergeScript, "/docker-config-out/config.json") - - // Verify docker-config-out volume mount exists on docker-auth-init - hasConfigOut := false - for _, vm := range dockerAuthInit.VolumeMounts { - if vm.Name == "kagent-docker-config" && vm.MountPath == "/docker-config-out" { - hasConfigOut = true - } - } - assert.True(t, hasConfigOut, "docker-auth-init should mount kagent-docker-config at /docker-config-out") + // No EmptyDir docker-config volume should exist. + for _, v := range deployment.Spec.Template.Spec.Volumes { + assert.NotEqual(t, "kagent-docker-config", v.Name, "kagent-docker-config EmptyDir volume must not exist") + } + + if tt.wantImagePullSecret { + // Script must contain the credential merge logic. + assert.Contains(t, script, "jq") + assert.Contains(t, script, ".dockerconfigjson") + assert.Contains(t, script, "/tmp/kagent-docker-config/config.json") + assert.Contains(t, script, "export DOCKER_CONFIG=/tmp/kagent-docker-config") - // Verify pull secret volumes and mounts are present on docker-auth-init require.NotNil(t, tt.agent.Spec.Skills) for _, ps := range tt.agent.Spec.Skills.ImagePullSecrets { volName := "pull-secret-" + ps.Name - // Volume on deployment + // Secret volume on the pod. hasPullSecretVol := false for _, v := range deployment.Spec.Template.Spec.Volumes { if v.Name == volName && v.Secret != nil && v.Secret.SecretName == ps.Name { hasPullSecretVol = true } } - assert.True(t, hasPullSecretVol, "pull-secret volume %q should exist", volName) + assert.True(t, hasPullSecretVol, "pull-secret volume %q should exist on the pod", volName) - // Mount on docker-auth-init + // Volume mounted on skills-init. hasPullSecretMount := false - for _, vm := range dockerAuthInit.VolumeMounts { + for _, vm := range skillsInitContainer.VolumeMounts { if vm.Name == volName && vm.MountPath == "/docker-secrets/"+ps.Name && vm.ReadOnly { hasPullSecretMount = true } } - assert.True(t, hasPullSecretMount, "docker-auth-init should mount pull-secret %q", volName) + assert.True(t, hasPullSecretMount, "skills-init should mount pull-secret %q at /docker-secrets/%s", volName, ps.Name) - // Merge script references this secret - assert.Contains(t, mergeScript, "/docker-secrets/"+ps.Name+"/.dockerconfigjson") + // Script references each secret by name. + assert.Contains(t, script, "/docker-secrets/"+ps.Name+"/.dockerconfigjson") } - - // Verify shared EmptyDir volume exists - hasDockerConfigVol := false - for _, v := range deployment.Spec.Template.Spec.Volumes { - if v.Name == "kagent-docker-config" { - hasDockerConfigVol = true - assert.NotNil(t, v.EmptyDir, "kagent-docker-config should be an EmptyDir volume") - } - } - assert.True(t, hasDockerConfigVol, "kagent-docker-config EmptyDir volume should exist") - - // Verify skills-init mounts the docker config and has DOCKER_CONFIG env - hasDockerMount := false - for _, vm := range skillsInitContainer.VolumeMounts { - if vm.Name == "kagent-docker-config" && vm.MountPath == "/.kagent/.docker" && vm.ReadOnly { - hasDockerMount = true - } - } - assert.True(t, hasDockerMount, "skills-init should mount kagent-docker-config at /.kagent/.docker") - - hasDockerConfigEnv := false - for _, e := range skillsInitContainer.Env { - if e.Name == "DOCKER_CONFIG" && e.Value == "/.kagent/.docker" { - hasDockerConfigEnv = true - } - } - assert.True(t, hasDockerConfigEnv, "skills-init should have DOCKER_CONFIG env var pointing to /.kagent/.docker") } else { - // No imagePullSecrets: no docker-auth-init, no DOCKER_CONFIG env, no docker config volumes - for _, c := range initContainers { - assert.NotEqual(t, "docker-auth-init", c.Name, "docker-auth-init should not exist without imagePullSecrets") - } - for _, e := range skillsInitContainer.Env { - assert.NotEqual(t, "DOCKER_CONFIG", e.Name, "DOCKER_CONFIG env should not be set without imagePullSecrets") - } + // No credential merge logic in the script. + assert.NotContains(t, script, "DOCKER_CONFIG") + assert.NotContains(t, script, "kagent-docker-config") + // No pull-secret volumes. for _, v := range deployment.Spec.Template.Spec.Volumes { - assert.NotEqual(t, "kagent-docker-config", v.Name, "kagent-docker-config volume should not exist") + assert.False(t, len(v.Name) > len("pull-secret-") && v.Name[:len("pull-secret-")] == "pull-secret-", + "no pull-secret volumes should exist without imagePullSecrets") } } }) diff --git a/go/core/internal/controller/translator/agent/skills-init.sh.tmpl b/go/core/internal/controller/translator/agent/skills-init.sh.tmpl index 4f0f3370e..4db51dd0d 100644 --- a/go/core/internal/controller/translator/agent/skills-init.sh.tmpl +++ b/go/core/internal/controller/translator/agent/skills-init.sh.tmpl @@ -1,4 +1,15 @@ set -e +{{- if .ImagePullSecrets }} +mkdir -p /tmp/kagent-docker-config +merged='{"auths":{}}' +{{- range .ImagePullSecrets }} +if [ -f /docker-secrets/{{ . }}/.dockerconfigjson ]; then + merged="$(printf '%s\n%s\n' "$merged" "$(cat /docker-secrets/{{ . }}/.dockerconfigjson)" | jq -s '.[0].auths * .[1].auths | {"auths": .}')" +fi +{{- end }} +printf '%s' "$merged" > /tmp/kagent-docker-config/config.json +export DOCKER_CONFIG=/tmp/kagent-docker-config +{{- end }} {{- if .AuthMountPath }} _auth_mount="$(cat <<'ENDVAL' {{ .AuthMountPath }} diff --git a/go/core/internal/controller/translator/agent/skills_unit_test.go b/go/core/internal/controller/translator/agent/skills_unit_test.go index d205ab957..6abc8c2dc 100644 --- a/go/core/internal/controller/translator/agent/skills_unit_test.go +++ b/go/core/internal/controller/translator/agent/skills_unit_test.go @@ -276,7 +276,7 @@ func Test_prepareSkillsInitData_duplicateNames(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - _, err := prepareSkillsInitData(tt.gitRefs, nil, tt.ociRefs, false) + _, err := prepareSkillsInitData(tt.gitRefs, nil, tt.ociRefs, false, nil) if tt.wantErr != "" { require.Error(t, err) assert.Contains(t, err.Error(), tt.wantErr) @@ -292,7 +292,7 @@ func Test_prepareSkillsInitData_pathTraversal(t *testing.T) { []v1alpha2.GitRepo{ {URL: "https://github.com/org/repo", Ref: "main", Path: "../escape"}, }, - nil, nil, false, + nil, nil, false, nil, ) require.Error(t, err) assert.Contains(t, err.Error(), "must not contain '..'") @@ -303,7 +303,7 @@ func Test_prepareSkillsInitData_absolutePath(t *testing.T) { []v1alpha2.GitRepo{ {URL: "https://github.com/org/repo", Ref: "main", Path: "/etc/passwd"}, }, - nil, nil, false, + nil, nil, false, nil, ) require.Error(t, err) assert.Contains(t, err.Error(), "must be relative") @@ -313,7 +313,7 @@ func Test_prepareSkillsInitData_authMountPath(t *testing.T) { data, err := prepareSkillsInitData( []v1alpha2.GitRepo{{URL: "https://github.com/org/repo", Ref: "main"}}, &corev1.LocalObjectReference{Name: "my-secret"}, - nil, false, + nil, false, nil, ) require.NoError(t, err) assert.Equal(t, "/git-auth", data.AuthMountPath) @@ -329,7 +329,7 @@ func Test_prepareSkillsInitData_sshHosts(t *testing.T) { }, &corev1.LocalObjectReference{Name: "ssh-secret"}, nil, - false, + false, nil, ) require.NoError(t, err) assert.Equal(t, []sshHostData{ @@ -346,7 +346,7 @@ func Test_prepareSkillsInitData_sshHostsDedupesDefaultPort(t *testing.T) { }, &corev1.LocalObjectReference{Name: "ssh-secret"}, nil, - false, + false, nil, ) require.NoError(t, err) assert.Equal(t, []sshHostData{ @@ -362,7 +362,7 @@ func Test_prepareSkillsInitData_noAuthSkipsSSHHosts(t *testing.T) { }, nil, // no auth secret nil, - false, + false, nil, ) require.NoError(t, err) assert.Empty(t, data.SSHHosts, "SSH hosts should not be collected when authSecretRef is nil") diff --git a/go/core/test/e2e/invoke_api_test.go b/go/core/test/e2e/invoke_api_test.go index b463f1da9..540f362ec 100644 --- a/go/core/test/e2e/invoke_api_test.go +++ b/go/core/test/e2e/invoke_api_test.go @@ -1159,7 +1159,7 @@ func TestE2ESkillImagePullSecrets(t *testing.T) { // Create a dummy dockerconfigjson secret. // The kind-registry is unauthenticated, so credentials don't matter — - // we're testing that the controller wires up the docker-auth-init container. + // we're testing that the controller embeds the credential merge logic into skills-init. dockerConfigJSON := `{"auths":{"kind-registry:5000":{"username":"user","password":"pass","auth":"dXNlcjpwYXNz"}}}` pullSecret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ @@ -1186,23 +1186,29 @@ func TestE2ESkillImagePullSecrets(t *testing.T) { }, }) - // Verify the Deployment has the docker-auth-init init container + // Verify the Deployment has exactly one init container: skills-init (credential merge is embedded in its script) deployment := &appsv1.Deployment{} require.NoError(t, cli.Get(t.Context(), client.ObjectKey{Name: agent.Name, Namespace: agent.Namespace}, deployment)) initContainers := deployment.Spec.Template.Spec.InitContainers - require.Len(t, initContainers, 2, "expected docker-auth-init + skills-init init containers") - require.Equal(t, "docker-auth-init", initContainers[0].Name) - require.Equal(t, "skills-init", initContainers[1].Name) + require.Len(t, initContainers, 1, "expected exactly one init container: skills-init") + require.Equal(t, "skills-init", initContainers[0].Name) - // Verify docker-auth-init mounts all pull secrets + // Verify skills-init mounts the pull secret volume and its script contains the merge logic + skillsInit := initContainers[0] var foundSecretMount bool - for _, vol := range initContainers[0].VolumeMounts { - if strings.Contains(vol.Name, "pull-secret") { + for _, vm := range skillsInit.VolumeMounts { + if strings.Contains(vm.Name, "pull-secret") { foundSecretMount = true break } } - require.True(t, foundSecretMount, "docker-auth-init should mount the pull secret volume") + require.True(t, foundSecretMount, "skills-init should mount the pull secret volume") + + require.Len(t, skillsInit.Command, 3) + script := skillsInit.Command[2] + require.Contains(t, script, "jq", "skills-init script should contain jq for credential merge") + require.Contains(t, script, ".dockerconfigjson", "skills-init script should reference .dockerconfigjson") + require.Contains(t, script, "/tmp/kagent-docker-config", "skills-init script should write merged config to /tmp") // Verify the agent works end-to-end with the skill a2aClient := setupA2AClient(t, agent)