From 0ed84a8da5c1e79ea64aff56c4ecf24374fc10bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20St=C3=A4bler?= Date: Wed, 11 Mar 2026 10:34:14 +0100 Subject: [PATCH 1/3] Pass --registry-insecure flag to Tekton pipeline tlsVerify parameter (#3484) When --registry-insecure is used, set tlsVerify=false in the Tekton pipeline to allow buildah to push to registries with self-signed certs. --- cmd/client.go | 5 +++-- cmd/deploy.go | 2 +- pkg/pipelines/tekton/pipelines_pac_provider.go | 2 +- pkg/pipelines/tekton/pipelines_provider.go | 9 ++++++++- pkg/pipelines/tekton/templates.go | 8 ++++---- pkg/pipelines/tekton/templates_test.go | 4 ++-- 6 files changed, 19 insertions(+), 11 deletions(-) diff --git a/cmd/client.go b/cmd/client.go index 9dea30f629..be08c74c87 100644 --- a/cmd/client.go +++ b/cmd/client.go @@ -61,7 +61,7 @@ func NewClient(cfg ClientConfig, options ...fn.Option) (*fn.Client, func()) { t = newTransport(cfg.InsecureSkipVerify) // may provide a custom impl which proxies c = newCredentialsProvider(config.Dir(), t, "") // for accessing registries d = newKnativeDeployer(cfg.Verbose) // default deployer (can be overridden via options) - pp = newTektonPipelinesProvider(c, cfg.Verbose) + pp = newTektonPipelinesProvider(c, cfg.Verbose, cfg.InsecureSkipVerify) o = []fn.Option{ // standard (shared) options for all commands fn.WithVerbose(cfg.Verbose), fn.WithTransport(t), @@ -142,10 +142,11 @@ func newCredentialsProvider(configPath string, t http.RoundTripper, authFilePath return creds.NewCredentialsProvider(configPath, options...) } -func newTektonPipelinesProvider(creds oci.CredentialsProvider, verbose bool) *tekton.PipelinesProvider { +func newTektonPipelinesProvider(creds oci.CredentialsProvider, verbose bool, registryInsecure bool) *tekton.PipelinesProvider { options := []tekton.Opt{ tekton.WithCredentialsProvider(creds), tekton.WithVerbose(verbose), + tekton.WithRegistryInsecure(registryInsecure), tekton.WithPipelineDecorator(deployDecorator{}), } diff --git a/cmd/deploy.go b/cmd/deploy.go index 772fd6162b..a35303c470 100644 --- a/cmd/deploy.go +++ b/cmd/deploy.go @@ -786,7 +786,7 @@ func (c deployConfig) clientOptions() ([]fn.Option, error) { // Override the pipelines provider to use custom credentials // This is needed for remote builds (deploy --remote) - o = append(o, fn.WithPipelinesProvider(newTektonPipelinesProvider(creds, c.Verbose))) + o = append(o, fn.WithPipelinesProvider(newTektonPipelinesProvider(creds, c.Verbose, c.RegistryInsecure))) // Add the appropriate deployer based on deploy type deployer := c.Deployer diff --git a/pkg/pipelines/tekton/pipelines_pac_provider.go b/pkg/pipelines/tekton/pipelines_pac_provider.go index c67f6cbb0b..542dc12a26 100644 --- a/pkg/pipelines/tekton/pipelines_pac_provider.go +++ b/pkg/pipelines/tekton/pipelines_pac_provider.go @@ -129,7 +129,7 @@ func (pp *PipelinesProvider) createLocalPACResources(ctx context.Context, f fn.F return err } - err = createPipelineRunTemplatePAC(f, labels) + err = createPipelineRunTemplatePAC(f, labels, pp.registryInsecure) if err != nil { return err } diff --git a/pkg/pipelines/tekton/pipelines_provider.go b/pkg/pipelines/tekton/pipelines_provider.go index 540f995c0d..c4131df0ef 100644 --- a/pkg/pipelines/tekton/pipelines_provider.go +++ b/pkg/pipelines/tekton/pipelines_provider.go @@ -52,6 +52,7 @@ type pacURLCallback = func() (string, error) type PipelinesProvider struct { verbose bool + registryInsecure bool getPacURL pacURLCallback credentialsProvider oci.CredentialsProvider decorator PipelineDecorator @@ -69,6 +70,12 @@ func WithVerbose(verbose bool) Opt { } } +func WithRegistryInsecure(insecure bool) Opt { + return func(pp *PipelinesProvider) { + pp.registryInsecure = insecure + } +} + func WithPipelineDecorator(decorator PipelineDecorator) Opt { return func(pp *PipelinesProvider) { pp.decorator = decorator @@ -203,7 +210,7 @@ func (pp *PipelinesProvider) Run(ctx context.Context, f fn.Function) (string, fn return "", f, fmt.Errorf("problem in creating secret: %v", err) } - err = createAndApplyPipelineRunTemplate(f, namespace, labels) + err = createAndApplyPipelineRunTemplate(f, namespace, labels, pp.registryInsecure) if err != nil { return "", f, fmt.Errorf("problem in creating pipeline run: %v", err) } diff --git a/pkg/pipelines/tekton/templates.go b/pkg/pipelines/tekton/templates.go index 149d9bb313..446a5633a4 100644 --- a/pkg/pipelines/tekton/templates.go +++ b/pkg/pipelines/tekton/templates.go @@ -128,7 +128,7 @@ func createPipelineTemplatePAC(f fn.Function, labels map[string]string) error { // createPipelineRunTemplatePAC creates a PipelineRun template used for PAC on-cluster build // it creates the resource in the project directory -func createPipelineRunTemplatePAC(f fn.Function, labels map[string]string) error { +func createPipelineRunTemplatePAC(f fn.Function, labels map[string]string, registryInsecure bool) error { contextDir := f.Build.Git.ContextDir if contextDir == "" && f.Build.Builder == builders.S2I { // TODO(lkingland): could instead update S2I to interpret empty string @@ -162,7 +162,7 @@ func createPipelineRunTemplatePAC(f fn.Function, labels map[string]string) error // Determine if TLS verification should be skipped tlsVerify := "true" - if isInsecureRegistry(f.Registry) { + if registryInsecure || isInsecureRegistry(f.Registry) { tlsVerify = "false" } @@ -325,7 +325,7 @@ func createAndApplyPipelineTemplate(f fn.Function, namespace string, labels map[ // createAndApplyPipelineRunTemplate creates and applies PipelineRun template for a standard on-cluster build // all resources are created on the fly, if there's a PipelineRun defined in the project directory, it is used instead -func createAndApplyPipelineRunTemplate(f fn.Function, namespace string, labels map[string]string) error { +func createAndApplyPipelineRunTemplate(f fn.Function, namespace string, labels map[string]string, registryInsecure bool) error { contextDir := f.Build.Git.ContextDir if contextDir == "" && f.Build.Builder == builders.S2I { // TODO(lkingland): could instead update S2I to interpret empty string @@ -359,7 +359,7 @@ func createAndApplyPipelineRunTemplate(f fn.Function, namespace string, labels m // Determine if TLS verification should be skipped tlsVerify := "true" - if isInsecureRegistry(f.Registry) { + if registryInsecure || isInsecureRegistry(f.Registry) { tlsVerify = "false" } diff --git a/pkg/pipelines/tekton/templates_test.go b/pkg/pipelines/tekton/templates_test.go index 14cfcfb292..378e09b334 100644 --- a/pkg/pipelines/tekton/templates_test.go +++ b/pkg/pipelines/tekton/templates_test.go @@ -147,7 +147,7 @@ func Test_createPipelineRunTemplatePAC(t *testing.T) { f.Image = "docker.io/alice/" + f.Name f.Registry = TestRegistry - err = createPipelineRunTemplatePAC(f, make(map[string]string)) + err = createPipelineRunTemplatePAC(f, make(map[string]string), false) if (err != nil) != tt.wantErr { t.Errorf("createPipelineRunTemplate() error = %v, wantErr %v", err, tt.wantErr) @@ -316,7 +316,7 @@ func Test_createAndApplyPipelineRunTemplate(t *testing.T) { f.Image = "docker.io/alice/" + f.Name f.Registry = TestRegistry - if err := createAndApplyPipelineRunTemplate(f, tt.namespace, tt.labels); (err != nil) != tt.wantErr { + if err := createAndApplyPipelineRunTemplate(f, tt.namespace, tt.labels, false); (err != nil) != tt.wantErr { t.Errorf("createAndApplyPipelineRunTemplate() error = %v, wantErr %v", err, tt.wantErr) } }) From 105fe564522ee8e29e54fad6e549702ce91d852f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20St=C3=A4bler?= Date: Fri, 13 Mar 2026 13:42:21 +0100 Subject: [PATCH 2/3] Remember `--registry-insecure` setting on recurring runs (#3490) * Revert "Pass --registry-insecure flag to Tekton pipeline tlsVerify parameter (#3484)" This reverts commit da21fa29868238a34d301f5e439a6dfe726a4c27. * Remember `--registry-insecure` setting on recurring runs * Run `make schema-generate` * Simplify parsing * Extend description of flag, that values are persited * Add warning, when registry changed but insecure-registry is still set to true * Fix typo * Rerun update-codegen * Cleanup test names * Add unit test for WarnRegistryInsecureChange * Run schema-generate * Update pkg/functions/client.go Co-authored-by: David Fridrich <49119790+gauron99@users.noreply.github.com> * Update pkg/functions/client.go Co-authored-by: David Fridrich <49119790+gauron99@users.noreply.github.com> * Update pkg/config/config.go Co-authored-by: David Fridrich <49119790+gauron99@users.noreply.github.com> * Update pkg/config/config.go Co-authored-by: David Fridrich <49119790+gauron99@users.noreply.github.com> * Fix format issues (goimport and gofmt) * Refactor registry-insecure unit test to make it available for the Deploy tests too * Refactor GlobalConfig.WarnRegistryInsecureChange() method to function in cmd package --------- Co-authored-by: David Fridrich <49119790+gauron99@users.noreply.github.com> --- cmd/build.go | 22 +- cmd/build_registry_insecure_test.go | 268 +++++++++++++++ cmd/client.go | 5 +- cmd/deploy.go | 8 +- cmd/deploy_test.go | 4 + docs/reference/func_build.md | 5 +- docs/reference/func_deploy.md | 2 +- pkg/config/config.go | 8 + pkg/functions/client.go | 40 +++ .../client_registry_insecure_test.go | 317 ++++++++++++++++++ pkg/functions/function.go | 4 + .../tekton/pipelines_pac_provider.go | 2 +- pkg/pipelines/tekton/pipelines_provider.go | 9 +- pkg/pipelines/tekton/templates.go | 8 +- pkg/pipelines/tekton/templates_test.go | 4 +- schema/func_yaml-schema.json | 4 + 16 files changed, 686 insertions(+), 24 deletions(-) create mode 100644 cmd/build_registry_insecure_test.go create mode 100644 pkg/functions/client_registry_insecure_test.go diff --git a/cmd/build.go b/cmd/build.go index b5d7c3803c..e6bd195bd5 100644 --- a/cmd/build.go +++ b/cmd/build.go @@ -3,6 +3,8 @@ package cmd import ( "errors" "fmt" + "io" + "os" "strings" "github.com/AlecAivazis/survey/v2" @@ -101,7 +103,7 @@ EXAMPLES fmt.Sprintf("Builder to use when creating the function's container. Currently supported builders are %s. ($FUNC_BUILDER)", KnownBuilders())) cmd.Flags().StringP("registry", "r", cfg.Registry, "Container registry + registry namespace. (ex 'ghcr.io/myuser'). The full image name is automatically determined using this along with function name. ($FUNC_REGISTRY)") - cmd.Flags().Bool("registry-insecure", cfg.RegistryInsecure, "Skip TLS certificate verification when communicating in HTTPS with the registry ($FUNC_REGISTRY_INSECURE)") + cmd.Flags().Bool("registry-insecure", cfg.RegistryInsecure, "Skip TLS certificate verification when communicating in HTTPS with the registry. The value is persisted over consecutive runs ($FUNC_REGISTRY_INSECURE)") cmd.Flags().String("registry-authfile", "", "Path to a authentication file containing registry credentials ($FUNC_REGISTRY_AUTHFILE)") // Function-Context Flags: @@ -170,6 +172,10 @@ func runBuild(cmd *cobra.Command, _ []string, newClient ClientFactory) (err erro if !f.Initialized() { return NewErrNotInitializedFromPath(f.Root, "build") } + + // Warn if registry changed but registryInsecure is still true + warnRegistryInsecureChange(os.Stderr, cfg.Registry, f) + f = cfg.Configure(f) // Returns an f updated with values from the config (flags, envs, etc) // Client @@ -206,6 +212,15 @@ func runBuild(cmd *cobra.Command, _ []string, newClient ClientFactory) (err erro return f.Stamp() } +// warnRegistryInsecureChange checks if the registry has changed but +// registryInsecure is still set to true, and prints a warning if so. +// This helps users avoid accidentally skipping TLS verification on a new registry. +func warnRegistryInsecureChange(w io.Writer, newRegistry string, f fn.Function) { + if f.Registry != "" && newRegistry != "" && f.Registry != newRegistry && f.RegistryInsecure { + fmt.Fprintf(w, "Warning: Registry changed from '%s' to '%s', but registryInsecure is still true. Consider setting --registry-insecure=false if the new registry requires TLS verification.\n", f.Registry, newRegistry) + } +} + type buildConfig struct { // Globals (builder, confirm, registry, verbose) config.Global @@ -435,7 +450,10 @@ func (c buildConfig) Validate(cmd *cobra.Command) (err error) { // image necessary for the target cluster, since the end product of a function // deployment is not the container, but rather the running service. func (c buildConfig) clientOptions() ([]fn.Option, error) { - o := []fn.Option{fn.WithRegistry(c.Registry)} + o := []fn.Option{ + fn.WithRegistry(c.Registry), + fn.WithRegistryInsecure(c.RegistryInsecure), + } t := newTransport(c.RegistryInsecure) creds := newCredentialsProvider(config.Dir(), t, c.RegistryAuthfile) diff --git a/cmd/build_registry_insecure_test.go b/cmd/build_registry_insecure_test.go new file mode 100644 index 0000000000..c3d0c628d3 --- /dev/null +++ b/cmd/build_registry_insecure_test.go @@ -0,0 +1,268 @@ +package cmd + +import ( + "bytes" + "strings" + "testing" + + "github.com/spf13/cobra" + fn "knative.dev/func/pkg/functions" + "knative.dev/func/pkg/mock" + . "knative.dev/func/pkg/testing" +) + +func TestBuild_RegistryInsecurePersists(t *testing.T) { + testRegistryInsecurePersists(NewBuildCmd, t) +} + +// testRegistryInsecurePersists ensures that the registryInsecure flag +// value is persisted to func.yaml and remembered across consecutive runs. +// See issue https://github.com/knative/func/issues/3489 +func testRegistryInsecurePersists(cmdFn func(factory ClientFactory) *cobra.Command, t *testing.T) { + root := FromTempDirectory(t) + + // Initialize a new function without registryInsecure set + f := fn.Function{ + Root: root, + Name: "myfunc", + Runtime: "go", + Registry: "example.com/alice", + } + if _, err := fn.New().Init(f); err != nil { + t.Fatal(err) + } + + var ( + builder = mock.NewBuilder() + pusher = mock.NewPusher() + ) + + // Test 1: Initial state - registryInsecure should be false + t.Run("initial state is false", func(t *testing.T) { + f, err := fn.NewFunction(root) + if err != nil { + t.Fatal(err) + } + + if f.RegistryInsecure { + t.Fatal("initial registryInsecure should be false") + } + }) + + // Test 2: Set registryInsecure to true with flag + t.Run("sets to true when flag passed", func(t *testing.T) { + cmd := cmdFn(NewTestClient( + fn.WithRegistry(TestRegistry), + fn.WithBuilder(builder), + fn.WithPusher(pusher), + )) + cmd.SetArgs([]string{"--registry-insecure=true"}) + + if err := cmd.Execute(); err != nil { + t.Fatal(err) + } + + // Load the function and verify registryInsecure is true + f, err := fn.NewFunction(root) + if err != nil { + t.Fatal(err) + } + + if !f.RegistryInsecure { + t.Fatal("registryInsecure should be true when flag passed, but was false") + } + }) + + // Test 3: Run build WITHOUT --registry-insecure flag + // Expected: registryInsecure should remain true (persisted value) + // This is the key test for issue #3489 + t.Run("persists true when flag not passed", func(t *testing.T) { + cmd := cmdFn(NewTestClient( + fn.WithRegistry(TestRegistry), + fn.WithBuilder(builder), + fn.WithPusher(pusher), + )) + cmd.SetArgs([]string{}) + + if err := cmd.Execute(); err != nil { + t.Fatal(err) + } + + // Load the function and verify registryInsecure is still true + f, err := fn.NewFunction(root) + if err != nil { + t.Fatal(err) + } + + if !f.RegistryInsecure { + t.Fatal("registryInsecure should be preserved as true, but was false") + } + }) + + // Test 4: Explicitly set --registry-insecure=false + // Expected: registryInsecure should be cleared (set to false) + t.Run("clears when flag set to false", func(t *testing.T) { + cmd := cmdFn(NewTestClient( + fn.WithRegistry(TestRegistry), + fn.WithBuilder(builder), + fn.WithPusher(pusher), + )) + cmd.SetArgs([]string{"--registry-insecure=false"}) + + if err := cmd.Execute(); err != nil { + t.Fatal(err) + } + + // Load the function and verify registryInsecure is false + f, err := fn.NewFunction(root) + if err != nil { + t.Fatal(err) + } + + if f.RegistryInsecure { + t.Fatal("registryInsecure should be false when flag set to false, but was true") + } + }) + + // Test 5: Run build again WITHOUT flag after clearing + // Expected: registryInsecure should stay false + t.Run("stays false when not set", func(t *testing.T) { + cmd := cmdFn(NewTestClient( + fn.WithRegistry(TestRegistry), + fn.WithBuilder(builder), + fn.WithPusher(pusher), + )) + cmd.SetArgs([]string{}) + + if err := cmd.Execute(); err != nil { + t.Fatal(err) + } + + // Load the function and verify registryInsecure is still false + f, err := fn.NewFunction(root) + if err != nil { + t.Fatal(err) + } + + if f.RegistryInsecure { + t.Fatal("registryInsecure should remain false, but was true") + } + }) + + // Test 6: Set back to true and verify multiple consecutive runs + t.Run("persists across multiple consecutive runs", func(t *testing.T) { + // First set it to true + cmd := cmdFn(NewTestClient( + fn.WithRegistry(TestRegistry), + fn.WithBuilder(builder), + fn.WithPusher(pusher), + )) + cmd.SetArgs([]string{"--registry-insecure=true"}) + + if err := cmd.Execute(); err != nil { + t.Fatal(err) + } + + // Run 3 times without the flag + for i := 0; i < 3; i++ { + cmd := cmdFn(NewTestClient( + fn.WithRegistry(TestRegistry), + fn.WithBuilder(builder), + fn.WithPusher(pusher), + )) + cmd.SetArgs([]string{}) + + if err := cmd.Execute(); err != nil { + t.Fatalf("build %d failed: %v", i+1, err) + } + + // Load and verify after each build + f, err := fn.NewFunction(root) + if err != nil { + t.Fatalf("loading function after build %d failed: %v", i+1, err) + } + + if !f.RegistryInsecure { + t.Fatalf("build %d: registryInsecure should be true, but was false", i+1) + } + } + }) +} + +// TestWarnRegistryInsecureChange ensures that the warning is printed when +// the registry changes but registryInsecure is still true. +func TestWarnRegistryInsecureChange(t *testing.T) { + tests := []struct { + name string + cfgRegistry string + funcRegistry string + funcInsecure bool + expectWarning bool + expectedMessage string + }{ + { + name: "no warning - registry not changed", + cfgRegistry: "example.com/alice", + funcRegistry: "example.com/alice", + funcInsecure: true, + expectWarning: false, + }, + { + name: "no warning - registryInsecure is false", + cfgRegistry: "example.com/bob", + funcRegistry: "example.com/alice", + funcInsecure: false, + expectWarning: false, + }, + { + name: "no warning - func registry is empty", + cfgRegistry: "example.com/bob", + funcRegistry: "", + funcInsecure: true, + expectWarning: false, + }, + { + name: "no warning - cfg registry is empty", + cfgRegistry: "", + funcRegistry: "example.com/alice", + funcInsecure: true, + expectWarning: false, + }, + { + name: "warning - registry changed and insecure is true", + cfgRegistry: "example.com/bob", + funcRegistry: "example.com/alice", + funcInsecure: true, + expectWarning: true, + expectedMessage: "Warning: Registry changed from 'example.com/alice' to 'example.com/bob', but registryInsecure is still true.", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + f := fn.Function{ + Registry: tt.funcRegistry, + RegistryInsecure: tt.funcInsecure, + } + + // Capture output + var buf bytes.Buffer + warnRegistryInsecureChange(&buf, tt.cfgRegistry, f) + + output := buf.String() + + if tt.expectWarning { + if output == "" { + t.Fatal("expected warning but got none") + } + if tt.expectedMessage != "" && !strings.Contains(output, tt.expectedMessage) { + t.Fatalf("expected message to contain '%s', got '%s'", tt.expectedMessage, output) + } + } else { + if output != "" { + t.Fatalf("expected no warning but got: %s", output) + } + } + }) + } +} diff --git a/cmd/client.go b/cmd/client.go index be08c74c87..9dea30f629 100644 --- a/cmd/client.go +++ b/cmd/client.go @@ -61,7 +61,7 @@ func NewClient(cfg ClientConfig, options ...fn.Option) (*fn.Client, func()) { t = newTransport(cfg.InsecureSkipVerify) // may provide a custom impl which proxies c = newCredentialsProvider(config.Dir(), t, "") // for accessing registries d = newKnativeDeployer(cfg.Verbose) // default deployer (can be overridden via options) - pp = newTektonPipelinesProvider(c, cfg.Verbose, cfg.InsecureSkipVerify) + pp = newTektonPipelinesProvider(c, cfg.Verbose) o = []fn.Option{ // standard (shared) options for all commands fn.WithVerbose(cfg.Verbose), fn.WithTransport(t), @@ -142,11 +142,10 @@ func newCredentialsProvider(configPath string, t http.RoundTripper, authFilePath return creds.NewCredentialsProvider(configPath, options...) } -func newTektonPipelinesProvider(creds oci.CredentialsProvider, verbose bool, registryInsecure bool) *tekton.PipelinesProvider { +func newTektonPipelinesProvider(creds oci.CredentialsProvider, verbose bool) *tekton.PipelinesProvider { options := []tekton.Opt{ tekton.WithCredentialsProvider(creds), tekton.WithVerbose(verbose), - tekton.WithRegistryInsecure(registryInsecure), tekton.WithPipelineDecorator(deployDecorator{}), } diff --git a/cmd/deploy.go b/cmd/deploy.go index a35303c470..bb47b0fdba 100644 --- a/cmd/deploy.go +++ b/cmd/deploy.go @@ -160,7 +160,7 @@ EXAMPLES fmt.Sprintf("Builder to use when creating the function's container. Currently supported builders are %s.", KnownBuilders())) cmd.Flags().StringP("registry", "r", cfg.Registry, "Container registry + registry namespace. (ex 'ghcr.io/myuser'). The full image name is automatically determined using this along with function name. ($FUNC_REGISTRY)") - cmd.Flags().Bool("registry-insecure", cfg.RegistryInsecure, "Skip TLS certificate verification when communicating in HTTPS with the registry ($FUNC_REGISTRY_INSECURE)") + cmd.Flags().Bool("registry-insecure", cfg.RegistryInsecure, "Skip TLS certificate verification when communicating in HTTPS with the registry. The value is persisted over consecutive runs ($FUNC_REGISTRY_INSECURE)") cmd.Flags().String("registry-authfile", "", "Path to a authentication file containing registry credentials ($FUNC_REGISTRY_AUTHFILE)") // Function-Context Flags: @@ -282,6 +282,10 @@ func runDeploy(cmd *cobra.Command, newClient ClientFactory) (err error) { if err = cfg.Validate(cmd); err != nil { return wrapValidateError(err, "deploy") } + + // Warn if registry changed but registryInsecure is still true + warnRegistryInsecureChange(cmd.OutOrStderr(), cfg.Registry, f) + if f, err = cfg.Configure(f); err != nil { // Updates f with deploy cfg return } @@ -786,7 +790,7 @@ func (c deployConfig) clientOptions() ([]fn.Option, error) { // Override the pipelines provider to use custom credentials // This is needed for remote builds (deploy --remote) - o = append(o, fn.WithPipelinesProvider(newTektonPipelinesProvider(creds, c.Verbose, c.RegistryInsecure))) + o = append(o, fn.WithPipelinesProvider(newTektonPipelinesProvider(creds, c.Verbose))) // Add the appropriate deployer based on deploy type deployer := c.Deployer diff --git a/cmd/deploy_test.go b/cmd/deploy_test.go index bdbdfbd260..efc68791bb 100644 --- a/cmd/deploy_test.go +++ b/cmd/deploy_test.go @@ -2346,3 +2346,7 @@ func TestDeploy_ValidDomain(t *testing.T) { }) } } + +func TestDeploy_RegistryInsecurePersists(t *testing.T) { + testRegistryInsecurePersists(NewDeployCmd, t) +} diff --git a/docs/reference/func_build.md b/docs/reference/func_build.md index 505f2a1709..d38c812377 100644 --- a/docs/reference/func_build.md +++ b/docs/reference/func_build.md @@ -64,12 +64,15 @@ func build -c, --confirm Prompt to confirm options interactively ($FUNC_CONFIRM) -h, --help help for build -i, --image string Full image name in the form [registry]/[namespace]/[name]:[tag] (optional). This option takes precedence over --registry ($FUNC_IMAGE) + --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) --platform string Optionally specify a target platform, for example "linux/amd64" when using the s2i build strategy -u, --push Attempt to push the function image to the configured registry after being successfully built -r, --registry string Container registry + registry namespace. (ex 'ghcr.io/myuser'). The full image name is automatically determined using this along with function name. ($FUNC_REGISTRY) --registry-authfile string Path to a authentication file containing registry credentials ($FUNC_REGISTRY_AUTHFILE) - --registry-insecure Skip TLS certificate verification when communicating in HTTPS with the registry ($FUNC_REGISTRY_INSECURE) + --registry-insecure Skip TLS certificate verification when communicating in HTTPS with the registry. The value is persisted over consecutive runs ($FUNC_REGISTRY_INSECURE) + --token string Token to use when pushing to the registry. ($FUNC_TOKEN) + --username string Username to use when pushing to the registry. ($FUNC_USERNAME) -v, --verbose Print verbose logs ($FUNC_VERBOSE) ``` diff --git a/docs/reference/func_deploy.md b/docs/reference/func_deploy.md index 2769a21fd8..403b5c95b3 100644 --- a/docs/reference/func_deploy.md +++ b/docs/reference/func_deploy.md @@ -134,7 +134,7 @@ func deploy --pvc-size string When triggering a remote deployment, set a custom volume size to allocate for the build operation ($FUNC_PVC_SIZE) -r, --registry string Container registry + registry namespace. (ex 'ghcr.io/myuser'). The full image name is automatically determined using this along with function name. ($FUNC_REGISTRY) --registry-authfile string Path to a authentication file containing registry credentials ($FUNC_REGISTRY_AUTHFILE) - --registry-insecure Skip TLS certificate verification when communicating in HTTPS with the registry ($FUNC_REGISTRY_INSECURE) + --registry-insecure Skip TLS certificate verification when communicating in HTTPS with the registry. The value is persisted over consecutive runs ($FUNC_REGISTRY_INSECURE) -R, --remote Trigger a remote deployment. Default is to deploy and build from the local system ($FUNC_REMOTE) --remote-storage-class string Specify a storage class to use for the volume on-cluster during remote builds --service-account string Service account to be used in the deployed function ($FUNC_SERVICE_ACCOUNT) diff --git a/pkg/config/config.go b/pkg/config/config.go index b07798c6fa..f26573346e 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -138,6 +138,10 @@ func (c Global) Apply(f fn.Function) Global { if f.Registry != "" { c.Registry = f.Registry } + // Unconditional because bool has no "empty value". Works because + // viper resolves the correct precedence via our defaulting. + c.RegistryInsecure = f.RegistryInsecure + return c } @@ -156,6 +160,10 @@ func (c Global) Configure(f fn.Function) fn.Function { if c.Registry != "" { f.Registry = c.Registry } + // Unconditional because bool has no "empty value". Works because + // viper resolves the correct precedence via our defaulting. + f.RegistryInsecure = c.RegistryInsecure + return f } diff --git a/pkg/functions/client.go b/pkg/functions/client.go index 40fedcfb0c..d8a7a11de7 100644 --- a/pkg/functions/client.go +++ b/pkg/functions/client.go @@ -74,6 +74,7 @@ type Client struct { describers []Describer // Describes function instances dnsProvider DNSProvider // Provider of DNS services registry string // default registry for OCI image tags + registryInsecure bool // skip TLS verification on registry repositories *Repositories // Repositories management templates *Templates // Templates management instances *InstanceRefs // Function Instances management @@ -372,6 +373,27 @@ func WithRegistry(registry string) Option { } } +// WithRegistryInsecure sets if the TLS verification of the registry should +// be skipped. +// +// NOTE: Unlike WithRegistry and other Options, which act as a fallback (function's value +// takes precedence when set), WithRegistryInsecure(true) WILL override the +// function's value unconditionally -> bool has no "unset" (unlike string's ""), +// so we cannot distinguish "not configured" from "explicitly set to false". +// As a result: +// - WithRegistryInsecure(true) → always sets f.RegistryInsecure = true +// - WithRegistryInsecure(false) → preserves the function's existing value +// +// NOTE-2: gauron99 - If we ever need to make this consistent with the rest of Options +// RegistryInsecure needs to become *bool in Function's struct & global.Config struct +// (nil == unset == empty value) and it would most likely require special use of +// cmd.Flags.Changed() and viper.IsSet() (non-standard for rest of commands) +func WithRegistryInsecure(insecure bool) Option { + return func(c *Client) { + c.registryInsecure = insecure + } +} + // WithTransport sets a custom transport to use internally. func WithTransport(t http.RoundTripper) Option { return func(c *Client) { @@ -690,6 +712,14 @@ func (c *Client) Build(ctx context.Context, f Function, options ...BuildOption) f.Registry = c.registry } + if c.registryInsecure { + // Unlike other With* functions (like WithRegistry) this takes bool. + // Bool has no "empty value" (like "" for string) therefore we always + // override if client's value is true. + // See WithRegistryInsecure definition comment for more info. + f.RegistryInsecure = true + } + // If no image name has been specified by user (--image), calculate. // Image name is stored on the function for later use by deploy, etc. var err error @@ -858,6 +888,11 @@ func (c *Client) RunPipeline(ctx context.Context, f Function) (string, Function, f.Registry = c.registry } + if c.registryInsecure { + // only override it, when given via the client + f.RegistryInsecure = true + } + // Build and deploy function using Pipeline return c.pipelinesProvider.Run(ctx, f) } @@ -872,6 +907,11 @@ func (c *Client) ConfigurePAC(ctx context.Context, f Function, metadata any) err f.Registry = c.registry } + if c.registryInsecure { + // only override it, when given via the client + f.RegistryInsecure = true + } + // If no image name has been yet defined (not yet built/deployed), calculate. // Image name is stored on the function for later use by deploy, etc. if f.Deploy.Image == "" { diff --git a/pkg/functions/client_registry_insecure_test.go b/pkg/functions/client_registry_insecure_test.go new file mode 100644 index 0000000000..6ded889a1d --- /dev/null +++ b/pkg/functions/client_registry_insecure_test.go @@ -0,0 +1,317 @@ +package functions_test + +import ( + "context" + "testing" + + fn "knative.dev/func/pkg/functions" + "knative.dev/func/pkg/mock" + . "knative.dev/func/pkg/testing" +) + +// TestClient_Build_RegistryInsecureFromClient ensures that when using the client API +// directly (not via CLI), the WithRegistryInsecure option correctly sets the value. +func TestClient_Build_RegistryInsecureFromClient(t *testing.T) { + root, cleanup := Mktemp(t) + defer cleanup() + + // Create client with registryInsecure option + client := fn.New( + fn.WithRegistry(TestRegistry), + fn.WithRegistryInsecure(true), + fn.WithBuilder(mock.NewBuilder()), + ) + + // Initialize a function without registryInsecure set + f, err := client.Init(fn.Function{ + Runtime: "go", + Root: root, + }) + if err != nil { + t.Fatal(err) + } + + // Build the function + f, err = client.Build(context.Background(), f) + if err != nil { + t.Fatal(err) + } + + // Verify registryInsecure was set by the client + if !f.RegistryInsecure { + t.Fatal("registryInsecure should be true from WithRegistryInsecure, but was false") + } + + // Write and reload to verify persistence + if err := f.Write(); err != nil { + t.Fatal(err) + } + + f2, err := fn.NewFunction(root) + if err != nil { + t.Fatal(err) + } + + if !f2.RegistryInsecure { + t.Fatal("registryInsecure should be persisted as true, but was false after reload") + } +} + +// TestClient_Build_RegistryInsecurePreservesExisting ensures that when a function +// already has registryInsecure set, the client respects the existing value. +func TestClient_Build_RegistryInsecurePreservesExisting(t *testing.T) { + root, cleanup := Mktemp(t) + defer cleanup() + + // Initialize a function with registryInsecure: true + client := fn.New( + fn.WithRegistry(TestRegistry), + fn.WithBuilder(mock.NewBuilder()), + ) + + f, err := client.Init(fn.Function{ + Runtime: "go", + Root: root, + RegistryInsecure: true, + }) + if err != nil { + t.Fatal(err) + } + + // Write the function + if err := f.Write(); err != nil { + t.Fatal(err) + } + + // Create a NEW client with registryInsecure=false (should not override) + client2 := fn.New( + fn.WithRegistry(TestRegistry), + fn.WithBuilder(mock.NewBuilder()), + fn.WithRegistryInsecure(false), + ) + + // Load the function + f, err = fn.NewFunction(root) + if err != nil { + t.Fatal(err) + } + + // Build with the new client + f, err = client2.Build(context.Background(), f) + if err != nil { + t.Fatal(err) + } + + // Verify the original value is preserved + if !f.RegistryInsecure { + t.Fatal("registryInsecure should be preserved as true, but was false") + } +} + +// TestClient_RunPipeline_RegistryInsecureFromClient ensures registryInsecure +// is set when using RunPipeline with the client API. +func TestClient_RunPipeline_RegistryInsecureFromClient(t *testing.T) { + root, cleanup := Mktemp(t) + defer cleanup() + + // Create client with registryInsecure option + pipeliner := mock.NewPipelinesProvider() + client := fn.New( + fn.WithRegistry(TestRegistry), + fn.WithRegistryInsecure(true), + fn.WithPipelinesProvider(pipeliner), + ) + + // Initialize a function with namespace + f, err := client.Init(fn.Function{ + Runtime: "go", + Root: root, + Namespace: "test-ns", + }) + if err != nil { + t.Fatal(err) + } + + // Run pipeline + _, f, err = client.RunPipeline(context.Background(), f) + if err != nil { + t.Fatal(err) + } + + // Verify registryInsecure was set + if !f.RegistryInsecure { + t.Fatal("registryInsecure should be true from WithRegistryInsecure, but was false") + } +} + +// TestClient_ConfigurePAC_RegistryInsecureFromClient ensures registryInsecure +// is set when using ConfigurePAC with the client API. +func TestClient_ConfigurePAC_RegistryInsecureFromClient(t *testing.T) { + root, cleanup := Mktemp(t) + defer cleanup() + + // Create client with registryInsecure option + pipeliner := mock.NewPipelinesProvider() + client := fn.New( + fn.WithRegistry(TestRegistry), + fn.WithRegistryInsecure(true), + fn.WithPipelinesProvider(pipeliner), + ) + + // Initialize a function + f, err := client.Init(fn.Function{ + Runtime: "go", + Root: root, + }) + if err != nil { + t.Fatal(err) + } + + // Configure PAC + if err = client.ConfigurePAC(context.Background(), f, nil); err != nil { + t.Fatal(err) + } + + // Load the function to verify it was written + f, err = fn.NewFunction(root) + if err != nil { + t.Fatal(err) + } + + // Verify registryInsecure was set + if !f.RegistryInsecure { + t.Fatal("registryInsecure should be true from WithRegistryInsecure, but was false") + } +} + +// TestClient_Build_RegistryInsecureDefaultFalse ensures that when neither +// the client nor function has registryInsecure set, it defaults to false. +func TestClient_Build_RegistryInsecureDefaultFalse(t *testing.T) { + root, cleanup := Mktemp(t) + defer cleanup() + + // Create client WITHOUT registryInsecure option + client := fn.New( + fn.WithRegistry(TestRegistry), + fn.WithBuilder(mock.NewBuilder()), + ) + + // Initialize a function without registryInsecure + f, err := client.Init(fn.Function{ + Runtime: "go", + Root: root, + }) + if err != nil { + t.Fatal(err) + } + + // Build the function + f, err = client.Build(context.Background(), f) + if err != nil { + t.Fatal(err) + } + + // Verify registryInsecure defaults to false + if f.RegistryInsecure { + t.Fatal("registryInsecure should default to false, but was true") + } + + // Write and reload + if err := f.Write(); err != nil { + t.Fatal(err) + } + + f2, err := fn.NewFunction(root) + if err != nil { + t.Fatal(err) + } + + // Verify it stays false after reload + if f2.RegistryInsecure { + t.Fatal("registryInsecure should remain false after reload, but was true") + } +} + +// TestClient_Build_RegistryInsecureToggle ensures the value can be toggled +// between true and false across multiple builds. +func TestClient_Build_RegistryInsecureToggle(t *testing.T) { + root, cleanup := Mktemp(t) + defer cleanup() + + // Initialize function + client := fn.New( + fn.WithRegistry(TestRegistry), + fn.WithBuilder(mock.NewBuilder()), + ) + + f, err := client.Init(fn.Function{ + Runtime: "go", + Root: root, + }) + if err != nil { + t.Fatal(err) + } + + // Set to true + f.RegistryInsecure = true + if err := f.Write(); err != nil { + t.Fatal(err) + } + + // Build and verify it stays true + f, err = fn.NewFunction(root) + if err != nil { + t.Fatal(err) + } + + f, err = client.Build(context.Background(), f) + if err != nil { + t.Fatal(err) + } + + if !f.RegistryInsecure { + t.Fatal("registryInsecure should be true") + } + + // Toggle to false + f.RegistryInsecure = false + if err := f.Write(); err != nil { + t.Fatal(err) + } + + // Build and verify it stays false + f, err = fn.NewFunction(root) + if err != nil { + t.Fatal(err) + } + + f, err = client.Build(context.Background(), f) + if err != nil { + t.Fatal(err) + } + + if f.RegistryInsecure { + t.Fatal("registryInsecure should be false after toggle") + } + + // Toggle back to true + f.RegistryInsecure = true + if err := f.Write(); err != nil { + t.Fatal(err) + } + + // Build and verify it's true again + f, err = fn.NewFunction(root) + if err != nil { + t.Fatal(err) + } + + f, err = client.Build(context.Background(), f) + if err != nil { + t.Fatal(err) + } + + if !f.RegistryInsecure { + t.Fatal("registryInsecure should be true after second toggle") + } +} diff --git a/pkg/functions/function.go b/pkg/functions/function.go index cc11bea760..01f2dd1d44 100644 --- a/pkg/functions/function.go +++ b/pkg/functions/function.go @@ -76,6 +76,10 @@ type Function struct { // [registry]/[user]. Registry string `yaml:"registry,omitempty"` + // RegistryInsecure defines if the TLS verification of the registry + // should be skipped. + RegistryInsecure bool `yaml:"registryInsecure,omitempty"` + // Image is the full OCI image tag in form: // [registry]/[namespace]/[name]:[tag] // example: diff --git a/pkg/pipelines/tekton/pipelines_pac_provider.go b/pkg/pipelines/tekton/pipelines_pac_provider.go index 542dc12a26..c67f6cbb0b 100644 --- a/pkg/pipelines/tekton/pipelines_pac_provider.go +++ b/pkg/pipelines/tekton/pipelines_pac_provider.go @@ -129,7 +129,7 @@ func (pp *PipelinesProvider) createLocalPACResources(ctx context.Context, f fn.F return err } - err = createPipelineRunTemplatePAC(f, labels, pp.registryInsecure) + err = createPipelineRunTemplatePAC(f, labels) if err != nil { return err } diff --git a/pkg/pipelines/tekton/pipelines_provider.go b/pkg/pipelines/tekton/pipelines_provider.go index c4131df0ef..540f995c0d 100644 --- a/pkg/pipelines/tekton/pipelines_provider.go +++ b/pkg/pipelines/tekton/pipelines_provider.go @@ -52,7 +52,6 @@ type pacURLCallback = func() (string, error) type PipelinesProvider struct { verbose bool - registryInsecure bool getPacURL pacURLCallback credentialsProvider oci.CredentialsProvider decorator PipelineDecorator @@ -70,12 +69,6 @@ func WithVerbose(verbose bool) Opt { } } -func WithRegistryInsecure(insecure bool) Opt { - return func(pp *PipelinesProvider) { - pp.registryInsecure = insecure - } -} - func WithPipelineDecorator(decorator PipelineDecorator) Opt { return func(pp *PipelinesProvider) { pp.decorator = decorator @@ -210,7 +203,7 @@ func (pp *PipelinesProvider) Run(ctx context.Context, f fn.Function) (string, fn return "", f, fmt.Errorf("problem in creating secret: %v", err) } - err = createAndApplyPipelineRunTemplate(f, namespace, labels, pp.registryInsecure) + err = createAndApplyPipelineRunTemplate(f, namespace, labels) if err != nil { return "", f, fmt.Errorf("problem in creating pipeline run: %v", err) } diff --git a/pkg/pipelines/tekton/templates.go b/pkg/pipelines/tekton/templates.go index 446a5633a4..2b279b8571 100644 --- a/pkg/pipelines/tekton/templates.go +++ b/pkg/pipelines/tekton/templates.go @@ -128,7 +128,7 @@ func createPipelineTemplatePAC(f fn.Function, labels map[string]string) error { // createPipelineRunTemplatePAC creates a PipelineRun template used for PAC on-cluster build // it creates the resource in the project directory -func createPipelineRunTemplatePAC(f fn.Function, labels map[string]string, registryInsecure bool) error { +func createPipelineRunTemplatePAC(f fn.Function, labels map[string]string) error { contextDir := f.Build.Git.ContextDir if contextDir == "" && f.Build.Builder == builders.S2I { // TODO(lkingland): could instead update S2I to interpret empty string @@ -162,7 +162,7 @@ func createPipelineRunTemplatePAC(f fn.Function, labels map[string]string, regis // Determine if TLS verification should be skipped tlsVerify := "true" - if registryInsecure || isInsecureRegistry(f.Registry) { + if f.RegistryInsecure || isInsecureRegistry(f.Registry) { tlsVerify = "false" } @@ -325,7 +325,7 @@ func createAndApplyPipelineTemplate(f fn.Function, namespace string, labels map[ // createAndApplyPipelineRunTemplate creates and applies PipelineRun template for a standard on-cluster build // all resources are created on the fly, if there's a PipelineRun defined in the project directory, it is used instead -func createAndApplyPipelineRunTemplate(f fn.Function, namespace string, labels map[string]string, registryInsecure bool) error { +func createAndApplyPipelineRunTemplate(f fn.Function, namespace string, labels map[string]string) error { contextDir := f.Build.Git.ContextDir if contextDir == "" && f.Build.Builder == builders.S2I { // TODO(lkingland): could instead update S2I to interpret empty string @@ -359,7 +359,7 @@ func createAndApplyPipelineRunTemplate(f fn.Function, namespace string, labels m // Determine if TLS verification should be skipped tlsVerify := "true" - if registryInsecure || isInsecureRegistry(f.Registry) { + if f.RegistryInsecure || isInsecureRegistry(f.Registry) { tlsVerify = "false" } diff --git a/pkg/pipelines/tekton/templates_test.go b/pkg/pipelines/tekton/templates_test.go index 378e09b334..14cfcfb292 100644 --- a/pkg/pipelines/tekton/templates_test.go +++ b/pkg/pipelines/tekton/templates_test.go @@ -147,7 +147,7 @@ func Test_createPipelineRunTemplatePAC(t *testing.T) { f.Image = "docker.io/alice/" + f.Name f.Registry = TestRegistry - err = createPipelineRunTemplatePAC(f, make(map[string]string), false) + err = createPipelineRunTemplatePAC(f, make(map[string]string)) if (err != nil) != tt.wantErr { t.Errorf("createPipelineRunTemplate() error = %v, wantErr %v", err, tt.wantErr) @@ -316,7 +316,7 @@ func Test_createAndApplyPipelineRunTemplate(t *testing.T) { f.Image = "docker.io/alice/" + f.Name f.Registry = TestRegistry - if err := createAndApplyPipelineRunTemplate(f, tt.namespace, tt.labels, false); (err != nil) != tt.wantErr { + if err := createAndApplyPipelineRunTemplate(f, tt.namespace, tt.labels); (err != nil) != tt.wantErr { t.Errorf("createAndApplyPipelineRunTemplate() error = %v, wantErr %v", err, tt.wantErr) } }) diff --git a/schema/func_yaml-schema.json b/schema/func_yaml-schema.json index eb8b637826..6ca1b782c7 100644 --- a/schema/func_yaml-schema.json +++ b/schema/func_yaml-schema.json @@ -181,6 +181,10 @@ "type": "string", "description": "Registry at which to store interstitial containers, in the form\n[registry]/[user]." }, + "registryInsecure": { + "type": "boolean", + "description": "RegistryInsecure defines if the TLS verification of the registry\nshould be skipped." + }, "image": { "type": "string", "description": "Image is the full OCI image tag in form:\n [registry]/[namespace]/[name]:[tag]\nexample:\n quay.io/alice/my.function.name\nRegistry is optional and is defaulted to DefaultRegistry\nexample:\n alice/my.function.name\nIf Image is provided, it overrides the default of concatenating\n\"Registry+Name:latest\" to derive the Image." From f8392648fe7219cbcfcc58e55dd6b77cbf9c94a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20St=C3=A4bler?= Date: Tue, 7 Apr 2026 07:55:11 +0200 Subject: [PATCH 3/3] Ran `./hack/update-codegen.sh` --- docs/reference/func_build.md | 3 --- 1 file changed, 3 deletions(-) diff --git a/docs/reference/func_build.md b/docs/reference/func_build.md index d38c812377..6b81b2283d 100644 --- a/docs/reference/func_build.md +++ b/docs/reference/func_build.md @@ -64,15 +64,12 @@ func build -c, --confirm Prompt to confirm options interactively ($FUNC_CONFIRM) -h, --help help for build -i, --image string Full image name in the form [registry]/[namespace]/[name]:[tag] (optional). This option takes precedence over --registry ($FUNC_IMAGE) - --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) --platform string Optionally specify a target platform, for example "linux/amd64" when using the s2i build strategy -u, --push Attempt to push the function image to the configured registry after being successfully built -r, --registry string Container registry + registry namespace. (ex 'ghcr.io/myuser'). The full image name is automatically determined using this along with function name. ($FUNC_REGISTRY) --registry-authfile string Path to a authentication file containing registry credentials ($FUNC_REGISTRY_AUTHFILE) --registry-insecure Skip TLS certificate verification when communicating in HTTPS with the registry. The value is persisted over consecutive runs ($FUNC_REGISTRY_INSECURE) - --token string Token to use when pushing to the registry. ($FUNC_TOKEN) - --username string Username to use when pushing to the registry. ($FUNC_USERNAME) -v, --verbose Print verbose logs ($FUNC_VERBOSE) ```