diff --git a/cmd/deploy.go b/cmd/deploy.go index c12fdf88ed..a2735b01b7 100644 --- a/cmd/deploy.go +++ b/cmd/deploy.go @@ -131,10 +131,10 @@ EXAMPLES SuggestFor: []string{"delpoy", "deplyo"}, PreRunE: bindEnv("build", "build-timestamp", "builder", "builder-image", "base-image", "confirm", "domain", "env", "git-branch", "git-dir", - "git-url", "image", "namespace", "path", "platform", "push", "pvc-size", - "service-account", "deployer", "registry", "registry-insecure", - "registry-authfile", "remote", "username", "password", "token", "verbose", - "remote-storage-class"), + "git-url", "image", "image-pull-secret", "namespace", "path", "platform", + "push", "pvc-size", "service-account", "deployer", "registry", + "registry-insecure", "registry-authfile", "remote", "username", "password", + "token", "verbose", "remote-storage-class"), RunE: func(cmd *cobra.Command, args []string) error { return runDeploy(cmd, newClient) }, @@ -195,6 +195,8 @@ EXAMPLES "When triggering a remote deployment, set a custom volume size to allocate for the build operation ($FUNC_PVC_SIZE)") cmd.Flags().String("service-account", f.Deploy.ServiceAccountName, "Service account to be used in the deployed function ($FUNC_SERVICE_ACCOUNT)") + cmd.Flags().String("image-pull-secret", f.Deploy.ImagePullSecret, + "Image pull secret to use when the function's image is in a private registry ($FUNC_IMAGE_PULL_SECRET)") cmd.Flags().String("deployer", f.Deploy.Deployer, fmt.Sprintf("Type of deployment to use: '%s' for Knative Service (default), '%s' for Kubernetes Deployment or '%s' for Deployment with a Keda HTTP scaler ($FUNC_DEPLOY_TYPE)", knative.KnativeDeployerName, k8s.KubernetesDeployerName, keda.KedaDeployerName)) // Static Flags: @@ -527,6 +529,9 @@ type deployConfig struct { //Service account to be used in deployed function ServiceAccountName string + // ImagePullSecret is the name of a secret for pulling the function image + ImagePullSecret string + // Deployer specifies the type of deployment: "knative" or "raw" Deployer string @@ -563,6 +568,7 @@ func newDeployConfig(cmd *cobra.Command) deployConfig { PVCSize: viper.GetString("pvc-size"), Timestamp: viper.GetBool("build-timestamp"), ServiceAccountName: viper.GetString("service-account"), + ImagePullSecret: viper.GetString("image-pull-secret"), Deployer: viper.GetString("deployer"), } // NOTE: .Env should be viper.GetStringSlice, but this returns unparsed @@ -598,6 +604,7 @@ func (c deployConfig) Configure(f fn.Function) (fn.Function, error) { f.Build.Git.Revision = c.GitBranch // TODO: should match; perhaps "refSpec" f.Build.RemoteStorageClass = c.RemoteStorageClass f.Deploy.ServiceAccountName = c.ServiceAccountName + f.Deploy.ImagePullSecret = c.ImagePullSecret f.Deploy.Deployer = c.Deployer f.Local.Remote = c.Remote diff --git a/cmd/deploy_test.go b/cmd/deploy_test.go index f413cdba74..2bd9e1880f 100644 --- a/cmd/deploy_test.go +++ b/cmd/deploy_test.go @@ -1831,6 +1831,87 @@ func TestDeploy_UnsetFlag(t *testing.T) { } } +// TestDeploy_ImagePullSecret ensures that the --image-pull-secret flag +// persists to func.yaml and can be cleared with an empty value. +func TestDeploy_ImagePullSecret(t *testing.T) { + root := FromTempDirectory(t) + + f := fn.Function{Runtime: "go", Root: root, Registry: TestRegistry} + _, err := fn.New().Init(f) + if err != nil { + t.Fatal(err) + } + + // Deploy with an image pull secret + cmd := NewDeployCmd(NewTestClient()) + cmd.SetArgs([]string{"--image-pull-secret=my-registry-secret"}) + if err := cmd.Execute(); err != nil { + t.Fatal(err) + } + + f, err = fn.NewFunction(root) + if err != nil { + t.Fatal(err) + } + if f.Deploy.ImagePullSecret != "my-registry-secret" { + t.Fatalf("expected imagePullSecret 'my-registry-secret', got '%v'", f.Deploy.ImagePullSecret) + } + + // Deploy again without the flag — value should be retained + cmd = NewDeployCmd(NewTestClient()) + if err := cmd.Execute(); err != nil { + t.Fatal(err) + } + + f, err = fn.NewFunction(root) + if err != nil { + t.Fatal(err) + } + if f.Deploy.ImagePullSecret != "my-registry-secret" { + t.Fatalf("expected imagePullSecret to be retained, got '%v'", f.Deploy.ImagePullSecret) + } + + // Deploy with empty value to clear it + cmd = NewDeployCmd(NewTestClient()) + cmd.SetArgs([]string{"--image-pull-secret="}) + if err := cmd.Execute(); err != nil { + t.Fatal(err) + } + + f, err = fn.NewFunction(root) + if err != nil { + t.Fatal(err) + } + if f.Deploy.ImagePullSecret != "" { + t.Fatalf("expected imagePullSecret to be cleared, got '%v'", f.Deploy.ImagePullSecret) + } +} + +// TestDeploy_ImagePullSecretFromEnv ensures FUNC_IMAGE_PULL_SECRET is respected. +func TestDeploy_ImagePullSecretFromEnv(t *testing.T) { + root := FromTempDirectory(t) + + f := fn.Function{Runtime: "go", Root: root, Registry: TestRegistry} + _, err := fn.New().Init(f) + if err != nil { + t.Fatal(err) + } + + t.Setenv("FUNC_IMAGE_PULL_SECRET", "env-secret") + cmd := NewDeployCmd(NewTestClient()) + if err := cmd.Execute(); err != nil { + t.Fatal(err) + } + + f, err = fn.NewFunction(root) + if err != nil { + t.Fatal(err) + } + if f.Deploy.ImagePullSecret != "env-secret" { + t.Fatalf("expected imagePullSecret 'env-secret' from env, got '%v'", f.Deploy.ImagePullSecret) + } +} + // Test_ValidateBuilder tests that the builder validation accepts the // set of known builders, and spot-checks an error is thrown for unknown. func Test_ValidateBuilder(t *testing.T) { diff --git a/docs/reference/func_deploy.md b/docs/reference/func_deploy.md index b97aab473b..630542c342 100644 --- a/docs/reference/func_deploy.md +++ b/docs/reference/func_deploy.md @@ -127,6 +127,7 @@ func deploy -g, --git-url string Repository url containing the function to build ($FUNC_GIT_URL) -h, --help help for deploy -i, --image string Full image name in the form [registry]/[namespace]/[name]:[tag]@[digest]. This option takes precedence over --registry. Specifying digest is optional, but if it is given, 'build' and 'push' phases are disabled. ($FUNC_IMAGE) + --image-pull-secret string Image pull secret to use when the function's image is in a private registry ($FUNC_IMAGE_PULL_SECRET) -n, --namespace string Deploy into a specific namespace. Will use the function's current namespace by default if already deployed, and the currently active context if it can be determined. ($FUNC_NAMESPACE) (default "default") --password string Password to use when pushing to the registry. ($FUNC_PASSWORD) -p, --path string Path to the function. Default is current directory ($FUNC_PATH) diff --git a/pkg/functions/function.go b/pkg/functions/function.go index 6144aeba04..09d2506273 100644 --- a/pkg/functions/function.go +++ b/pkg/functions/function.go @@ -218,6 +218,11 @@ type DeploySpec struct { // More info: https://kubernetes.io/docs/tasks/configure-pod-container/configure-service-account/ ServiceAccountName string `yaml:"serviceAccountName,omitempty"` + // ImagePullSecret is the name of a Secret in the same namespace used + // for pulling the function's container image from a private registry. + // More info: https://kubernetes.io/docs/tasks/configure-pod-container/pull-image-private-registry/ + ImagePullSecret string `yaml:"imagePullSecret,omitempty"` + // Deployer specifies the type of deployment to use: "knative", "raw" or "keda" // Defaults to "knative" for backwards compatibility Deployer string `yaml:"deployer,omitempty" jsonschema:"enum=knative,enum=raw,enum=keda"` diff --git a/pkg/k8s/deployer.go b/pkg/k8s/deployer.go index 901c728e3f..1412d7ba96 100644 --- a/pkg/k8s/deployer.go +++ b/pkg/k8s/deployer.go @@ -432,6 +432,7 @@ func (d *Deployer) generateDeployment(f fn.Function, namespace string, daprInsta Spec: corev1.PodSpec{ Containers: []corev1.Container{container}, ServiceAccountName: f.Deploy.ServiceAccountName, + ImagePullSecrets: ImagePullSecrets(f.Deploy.ImagePullSecret), Volumes: volumes, }, }, @@ -478,7 +479,7 @@ func (d *Deployer) generateService(f fn.Function, namespace string, daprInstalle // CheckResourcesArePresent returns error if Secrets or ConfigMaps // referenced in input sets are not deployed on the cluster in the specified namespace -func CheckResourcesArePresent(ctx context.Context, namespace string, referencedSecrets, referencedConfigMaps, referencedPVCs *sets.Set[string], referencedServiceAccount string) error { +func CheckResourcesArePresent(ctx context.Context, namespace string, referencedSecrets, referencedConfigMaps, referencedPVCs *sets.Set[string], referencedServiceAccount, imagePullSecret string) error { errMsg := "" for s := range *referencedSecrets { _, err := GetSecret(ctx, s, namespace) @@ -513,6 +514,13 @@ func CheckResourcesArePresent(ctx context.Context, namespace string, referencedS } } + if imagePullSecret != "" { + _, err := GetSecret(ctx, imagePullSecret, namespace) + if err != nil { + errMsg += fmt.Sprintf(" referenced image pull Secret \"%s\" is not present in namespace \"%s\"\n", imagePullSecret, namespace) + } + } + if errMsg != "" { return fmt.Errorf("error(s) while validating resources:\n%s", errMsg) } @@ -520,6 +528,15 @@ func CheckResourcesArePresent(ctx context.Context, namespace string, referencedS return nil } +// ImagePullSecrets converts a secret name to a slice of LocalObjectReference +// suitable for use in a PodSpec. Returns nil if the name is empty. +func ImagePullSecrets(secret string) []corev1.LocalObjectReference { + if secret == "" { + return nil + } + return []corev1.LocalObjectReference{{Name: secret}} +} + // SetHealthEndpoints configures health probes for a container func SetHealthEndpoints(f fn.Function, container *corev1.Container) { livenessPath := DefaultLivenessEndpoint diff --git a/pkg/k8s/deployer_test.go b/pkg/k8s/deployer_test.go index c173b8c8c0..52611baaa3 100644 --- a/pkg/k8s/deployer_test.go +++ b/pkg/k8s/deployer_test.go @@ -90,6 +90,64 @@ func Test_processValue(t *testing.T) { } } +func Test_ImagePullSecrets(t *testing.T) { + t.Run("empty secret returns nil", func(t *testing.T) { + refs := ImagePullSecrets("") + if refs != nil { + t.Errorf("expected nil, got %v", refs) + } + }) + + t.Run("non-empty secret returns single reference", func(t *testing.T) { + refs := ImagePullSecrets("my-secret") + if len(refs) != 1 { + t.Fatalf("expected 1 reference, got %d", len(refs)) + } + if refs[0].Name != "my-secret" { + t.Errorf("expected name 'my-secret', got '%s'", refs[0].Name) + } + }) +} + +func Test_generateDeployment_ImagePullSecret(t *testing.T) { + d := &Deployer{} + + t.Run("with image pull secret", func(t *testing.T) { + f := fn.Function{ + Name: "test-func", + Deploy: fn.DeploySpec{ + Image: "registry.example.com/test:latest", + ImagePullSecret: "my-registry-secret", + }, + } + deployment, err := d.generateDeployment(f, "default", false) + if err != nil { + t.Fatal(err) + } + secrets := deployment.Spec.Template.Spec.ImagePullSecrets + if len(secrets) != 1 || secrets[0].Name != "my-registry-secret" { + t.Errorf("expected ImagePullSecrets [{my-registry-secret}], got %v", secrets) + } + }) + + t.Run("without image pull secret", func(t *testing.T) { + f := fn.Function{ + Name: "test-func", + Deploy: fn.DeploySpec{ + Image: "registry.example.com/test:latest", + }, + } + deployment, err := d.generateDeployment(f, "default", false) + if err != nil { + t.Fatal(err) + } + secrets := deployment.Spec.Template.Spec.ImagePullSecrets + if secrets != nil { + t.Errorf("expected no ImagePullSecrets, got %v", secrets) + } + }) +} + // Tests for generateTriggerName func TestGenerateTriggerName_Deterministic(t *testing.T) { diff --git a/pkg/knative/deployer.go b/pkg/knative/deployer.go index b0d8d35f2c..4bc49c9b4a 100644 --- a/pkg/knative/deployer.go +++ b/pkg/knative/deployer.go @@ -173,12 +173,14 @@ func (d *Deployer) Deploy(ctx context.Context, f fn.Function) (fn.DeploymentResu defer func(t fnhttp.RoundTripCloser) { _ = t.Close() }(t) - if err = checkPullPermissions(ctx, k8sClient.CoreV1(), t, f.Deploy.Image, namespace); err != nil { + if err = checkPullPermissions(ctx, k8sClient.CoreV1(), t, f.Deploy.Image, namespace, f.Deploy.ImagePullSecret); err != nil { msg := fmt.Sprintf("warning: error while checking pull secrets: %v", err) switch { case stdErrors.Is(err, errPullSecretNotFound): msg = `warning: pull secrets not detected, the cluster **may** fail to pull the image -consider setting up the pull secrets and linking it to the default service account, sample setup: +consider using the --image-pull-secret flag, or setting up pull secrets manually: + $ func deploy --image-pull-secret + or: $ kubectl create secret docker-registry sample-secret --docker-server --docker-username --docker-password $ kubectl patch serviceaccount default -p '{"imagePullSecrets": [{"name": "sample-secret"}]}'` case stdErrors.Is(err, errOnlyIncorrectPullSecretFound): @@ -215,7 +217,7 @@ consider setting up the pull secrets and linking it to the default service accou return fn.DeploymentResult{}, err } - err = k8s.CheckResourcesArePresent(ctx, namespace, &referencedSecrets, &referencedConfigMaps, &referencedPVCs, f.Deploy.ServiceAccountName) + err = k8s.CheckResourcesArePresent(ctx, namespace, &referencedSecrets, &referencedConfigMaps, &referencedPVCs, f.Deploy.ServiceAccountName, f.Deploy.ImagePullSecret) if err != nil { err = fmt.Errorf("knative deployer failed to generate the Knative Service: %v", err) return fn.DeploymentResult{}, err @@ -317,7 +319,7 @@ consider setting up the pull secrets and linking it to the default service accou return fn.DeploymentResult{}, err } - err = k8s.CheckResourcesArePresent(ctx, namespace, &referencedSecrets, &referencedConfigMaps, &referencedPVCs, f.Deploy.ServiceAccountName) + err = k8s.CheckResourcesArePresent(ctx, namespace, &referencedSecrets, &referencedConfigMaps, &referencedPVCs, f.Deploy.ServiceAccountName, f.Deploy.ImagePullSecret) if err != nil { err = fmt.Errorf("knative deployer failed to update the Knative Service: %v", err) return fn.DeploymentResult{}, err @@ -469,6 +471,7 @@ func generateNewService(f fn.Function, decorator deployer.DeployDecorator, daprI container, }, ServiceAccountName: f.Deploy.ServiceAccountName, + ImagePullSecrets: k8s.ImagePullSecrets(f.Deploy.ImagePullSecret), Volumes: newVolumes, }, }, @@ -559,6 +562,7 @@ func updateService(f fn.Function, previousService *servingv1.Service, newEnv []c cp.VolumeMounts = newVolumeMounts service.Spec.Template.Spec.Volumes = newVolumes service.Spec.Template.Spec.ServiceAccountName = f.Deploy.ServiceAccountName + service.Spec.Template.Spec.ImagePullSecrets = k8s.ImagePullSecrets(f.Deploy.ImagePullSecret) return service, nil } } @@ -664,7 +668,7 @@ func UsesKnativeDeployer(annotations map[string]string) bool { return !ok || deployer == KnativeDeployerName } -func checkPullPermissions(ctx context.Context, core v1.CoreV1Interface, trans http.RoundTripper, img, ns string) error { +func checkPullPermissions(ctx context.Context, core v1.CoreV1Interface, trans http.RoundTripper, img, ns, imagePullSecret string) error { ref, err := name.ParseReference(img) if err != nil { return fmt.Errorf("failed to parse image %q: %w", img, err) @@ -679,11 +683,35 @@ func checkPullPermissions(ctx context.Context, core v1.CoreV1Interface, trans ht return err } + var incorrectCredentialsFound bool + + // check function-level image pull secret first + if imagePullSecret != "" { + sc, err := core.Secrets(ns).Get(ctx, imagePullSecret, metav1.GetOptions{}) + if err == nil { + cf, err := secretToConfigFile(sc) + if err == nil { + _, err = remote.Head(ref, + remote.WithContext(ctx), + remote.WithTransport(trans), + remote.WithAuthFromKeychain(configFileKeychain{cf: cf}), + ) + if err == nil { + return nil + } + if !isUnauthorized(err) && !stdErrors.Is(err, errPullSecretNotFound) { + return err + } + incorrectCredentialsFound = true + } + } + } + + // fall back to default ServiceAccount's image pull secrets sa, err := core.ServiceAccounts(ns).Get(ctx, "default", metav1.GetOptions{}) if err != nil { return fmt.Errorf("failed to get default ServiceAccount: %w", err) } - var incorrectCredentialsFound bool for _, secret := range sa.ImagePullSecrets { sc, err := core.Secrets(ns).Get(ctx, secret.Name, metav1.GetOptions{}) if err != nil { diff --git a/pkg/knative/deployer_test.go b/pkg/knative/deployer_test.go index 2de1d2d4be..a3b2aab85f 100644 --- a/pkg/knative/deployer_test.go +++ b/pkg/knative/deployer_test.go @@ -150,7 +150,7 @@ func TestCheckPullPermissions(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - err = checkPullPermissions(t.Context(), tt.core, trans, tt.image, "default") + err = checkPullPermissions(t.Context(), tt.core, trans, tt.image, "default", "") p := tt.errPred if p == nil { p = func(err error) bool { @@ -165,6 +165,50 @@ func TestCheckPullPermissions(t *testing.T) { } +func TestCheckPullPermissions_FunctionImagePullSecret(t *testing.T) { + secretA := createSecret(securedRegistry, + username, password, + corev1.SecretTypeDockerConfigJson, + ) + + trans := setupRegistry(t) + + t.Run("function-level secret with correct credentials", func(t *testing.T) { + // The secret is registered in the cluster but NOT on the ServiceAccount. + // It's provided as a function-level image pull secret. + sa := &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{Name: "default", Namespace: "default"}, + } + coreClient := fake.NewClientset(sa, secretA).CoreV1() + + err := checkPullPermissions(t.Context(), coreClient, trans, securedImage, "default", secretA.Name) + if err != nil { + t.Errorf("expected no error with valid function-level pull secret, got: %v", err) + } + }) + + t.Run("function-level secret with incorrect credentials", func(t *testing.T) { + badSecret := createSecret(securedRegistry, username, "wrong-password", corev1.SecretTypeDockerConfigJson) + sa := &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{Name: "default", Namespace: "default"}, + } + coreClient := fake.NewClientset(sa, badSecret).CoreV1() + + err := checkPullPermissions(t.Context(), coreClient, trans, securedImage, "default", badSecret.Name) + if !errors.Is(err, errOnlyIncorrectPullSecretFound) { + t.Errorf("expected errOnlyIncorrectPullSecretFound, got: %v", err) + } + }) + + t.Run("no function-level secret falls back to SA", func(t *testing.T) { + coreClient := core(secretA) + err := checkPullPermissions(t.Context(), coreClient, trans, securedImage, "default", "") + if err != nil { + t.Errorf("expected no error when SA has valid pull secret, got: %v", err) + } + }) +} + var secretCounter int32 func createSecret(reg, uname, pwd string, typ corev1.SecretType) *corev1.Secret { diff --git a/schema/func_yaml-schema.json b/schema/func_yaml-schema.json index 138f9d7ed7..762abdfe47 100644 --- a/schema/func_yaml-schema.json +++ b/schema/func_yaml-schema.json @@ -107,6 +107,10 @@ "type": "string", "description": "ServiceAccountName is the name of the service account used for the\nfunction pod. The service account must exist in the namespace to\nsucceed.\nMore info: https://kubernetes.io/docs/tasks/configure-pod-container/configure-service-account/" }, + "imagePullSecret": { + "type": "string", + "description": "ImagePullSecret is the name of a Secret in the same namespace used\nfor pulling the function's container image from a private registry.\nMore info: https://kubernetes.io/docs/tasks/configure-pod-container/pull-image-private-registry/" + }, "deployer": { "enum": [ "knative",