Skip to content

Commit efeae04

Browse files
creydrgauron99
andauthored
[release-1.21] Remember --registry-insecure setting on recurring runs (#3574)
* 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. * Remember `--registry-insecure` setting on recurring runs (#3490) * Revert "Pass --registry-insecure flag to Tekton pipeline tlsVerify parameter (#3484)" This reverts commit da21fa2. * 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> * Ran `./hack/update-codegen.sh` --------- Co-authored-by: David Fridrich <49119790+gauron99@users.noreply.github.com>
1 parent 32c0b4a commit efeae04

File tree

12 files changed

+674
-7
lines changed

12 files changed

+674
-7
lines changed

cmd/build.go

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ package cmd
33
import (
44
"errors"
55
"fmt"
6+
"io"
7+
"os"
68
"strings"
79

810
"github.com/AlecAivazis/survey/v2"
@@ -101,7 +103,7 @@ EXAMPLES
101103
fmt.Sprintf("Builder to use when creating the function's container. Currently supported builders are %s. ($FUNC_BUILDER)", KnownBuilders()))
102104
cmd.Flags().StringP("registry", "r", cfg.Registry,
103105
"Container registry + registry namespace. (ex 'ghcr.io/myuser'). The full image name is automatically determined using this along with function name. ($FUNC_REGISTRY)")
104-
cmd.Flags().Bool("registry-insecure", cfg.RegistryInsecure, "Skip TLS certificate verification when communicating in HTTPS with the registry ($FUNC_REGISTRY_INSECURE)")
106+
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)")
105107
cmd.Flags().String("registry-authfile", "", "Path to a authentication file containing registry credentials ($FUNC_REGISTRY_AUTHFILE)")
106108

107109
// Function-Context Flags:
@@ -170,6 +172,10 @@ func runBuild(cmd *cobra.Command, _ []string, newClient ClientFactory) (err erro
170172
if !f.Initialized() {
171173
return NewErrNotInitializedFromPath(f.Root, "build")
172174
}
175+
176+
// Warn if registry changed but registryInsecure is still true
177+
warnRegistryInsecureChange(os.Stderr, cfg.Registry, f)
178+
173179
f = cfg.Configure(f) // Returns an f updated with values from the config (flags, envs, etc)
174180

175181
// Client
@@ -206,6 +212,15 @@ func runBuild(cmd *cobra.Command, _ []string, newClient ClientFactory) (err erro
206212
return f.Stamp()
207213
}
208214

215+
// warnRegistryInsecureChange checks if the registry has changed but
216+
// registryInsecure is still set to true, and prints a warning if so.
217+
// This helps users avoid accidentally skipping TLS verification on a new registry.
218+
func warnRegistryInsecureChange(w io.Writer, newRegistry string, f fn.Function) {
219+
if f.Registry != "" && newRegistry != "" && f.Registry != newRegistry && f.RegistryInsecure {
220+
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)
221+
}
222+
}
223+
209224
type buildConfig struct {
210225
// Globals (builder, confirm, registry, verbose)
211226
config.Global
@@ -435,7 +450,10 @@ func (c buildConfig) Validate(cmd *cobra.Command) (err error) {
435450
// image necessary for the target cluster, since the end product of a function
436451
// deployment is not the container, but rather the running service.
437452
func (c buildConfig) clientOptions() ([]fn.Option, error) {
438-
o := []fn.Option{fn.WithRegistry(c.Registry)}
453+
o := []fn.Option{
454+
fn.WithRegistry(c.Registry),
455+
fn.WithRegistryInsecure(c.RegistryInsecure),
456+
}
439457

440458
t := newTransport(c.RegistryInsecure)
441459
creds := newCredentialsProvider(config.Dir(), t, c.RegistryAuthfile)
Lines changed: 268 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,268 @@
1+
package cmd
2+
3+
import (
4+
"bytes"
5+
"strings"
6+
"testing"
7+
8+
"github.com/spf13/cobra"
9+
fn "knative.dev/func/pkg/functions"
10+
"knative.dev/func/pkg/mock"
11+
. "knative.dev/func/pkg/testing"
12+
)
13+
14+
func TestBuild_RegistryInsecurePersists(t *testing.T) {
15+
testRegistryInsecurePersists(NewBuildCmd, t)
16+
}
17+
18+
// testRegistryInsecurePersists ensures that the registryInsecure flag
19+
// value is persisted to func.yaml and remembered across consecutive runs.
20+
// See issue https://github.com/knative/func/issues/3489
21+
func testRegistryInsecurePersists(cmdFn func(factory ClientFactory) *cobra.Command, t *testing.T) {
22+
root := FromTempDirectory(t)
23+
24+
// Initialize a new function without registryInsecure set
25+
f := fn.Function{
26+
Root: root,
27+
Name: "myfunc",
28+
Runtime: "go",
29+
Registry: "example.com/alice",
30+
}
31+
if _, err := fn.New().Init(f); err != nil {
32+
t.Fatal(err)
33+
}
34+
35+
var (
36+
builder = mock.NewBuilder()
37+
pusher = mock.NewPusher()
38+
)
39+
40+
// Test 1: Initial state - registryInsecure should be false
41+
t.Run("initial state is false", func(t *testing.T) {
42+
f, err := fn.NewFunction(root)
43+
if err != nil {
44+
t.Fatal(err)
45+
}
46+
47+
if f.RegistryInsecure {
48+
t.Fatal("initial registryInsecure should be false")
49+
}
50+
})
51+
52+
// Test 2: Set registryInsecure to true with flag
53+
t.Run("sets to true when flag passed", func(t *testing.T) {
54+
cmd := cmdFn(NewTestClient(
55+
fn.WithRegistry(TestRegistry),
56+
fn.WithBuilder(builder),
57+
fn.WithPusher(pusher),
58+
))
59+
cmd.SetArgs([]string{"--registry-insecure=true"})
60+
61+
if err := cmd.Execute(); err != nil {
62+
t.Fatal(err)
63+
}
64+
65+
// Load the function and verify registryInsecure is true
66+
f, err := fn.NewFunction(root)
67+
if err != nil {
68+
t.Fatal(err)
69+
}
70+
71+
if !f.RegistryInsecure {
72+
t.Fatal("registryInsecure should be true when flag passed, but was false")
73+
}
74+
})
75+
76+
// Test 3: Run build WITHOUT --registry-insecure flag
77+
// Expected: registryInsecure should remain true (persisted value)
78+
// This is the key test for issue #3489
79+
t.Run("persists true when flag not passed", func(t *testing.T) {
80+
cmd := cmdFn(NewTestClient(
81+
fn.WithRegistry(TestRegistry),
82+
fn.WithBuilder(builder),
83+
fn.WithPusher(pusher),
84+
))
85+
cmd.SetArgs([]string{})
86+
87+
if err := cmd.Execute(); err != nil {
88+
t.Fatal(err)
89+
}
90+
91+
// Load the function and verify registryInsecure is still true
92+
f, err := fn.NewFunction(root)
93+
if err != nil {
94+
t.Fatal(err)
95+
}
96+
97+
if !f.RegistryInsecure {
98+
t.Fatal("registryInsecure should be preserved as true, but was false")
99+
}
100+
})
101+
102+
// Test 4: Explicitly set --registry-insecure=false
103+
// Expected: registryInsecure should be cleared (set to false)
104+
t.Run("clears when flag set to false", func(t *testing.T) {
105+
cmd := cmdFn(NewTestClient(
106+
fn.WithRegistry(TestRegistry),
107+
fn.WithBuilder(builder),
108+
fn.WithPusher(pusher),
109+
))
110+
cmd.SetArgs([]string{"--registry-insecure=false"})
111+
112+
if err := cmd.Execute(); err != nil {
113+
t.Fatal(err)
114+
}
115+
116+
// Load the function and verify registryInsecure is false
117+
f, err := fn.NewFunction(root)
118+
if err != nil {
119+
t.Fatal(err)
120+
}
121+
122+
if f.RegistryInsecure {
123+
t.Fatal("registryInsecure should be false when flag set to false, but was true")
124+
}
125+
})
126+
127+
// Test 5: Run build again WITHOUT flag after clearing
128+
// Expected: registryInsecure should stay false
129+
t.Run("stays false when not set", func(t *testing.T) {
130+
cmd := cmdFn(NewTestClient(
131+
fn.WithRegistry(TestRegistry),
132+
fn.WithBuilder(builder),
133+
fn.WithPusher(pusher),
134+
))
135+
cmd.SetArgs([]string{})
136+
137+
if err := cmd.Execute(); err != nil {
138+
t.Fatal(err)
139+
}
140+
141+
// Load the function and verify registryInsecure is still false
142+
f, err := fn.NewFunction(root)
143+
if err != nil {
144+
t.Fatal(err)
145+
}
146+
147+
if f.RegistryInsecure {
148+
t.Fatal("registryInsecure should remain false, but was true")
149+
}
150+
})
151+
152+
// Test 6: Set back to true and verify multiple consecutive runs
153+
t.Run("persists across multiple consecutive runs", func(t *testing.T) {
154+
// First set it to true
155+
cmd := cmdFn(NewTestClient(
156+
fn.WithRegistry(TestRegistry),
157+
fn.WithBuilder(builder),
158+
fn.WithPusher(pusher),
159+
))
160+
cmd.SetArgs([]string{"--registry-insecure=true"})
161+
162+
if err := cmd.Execute(); err != nil {
163+
t.Fatal(err)
164+
}
165+
166+
// Run 3 times without the flag
167+
for i := 0; i < 3; i++ {
168+
cmd := cmdFn(NewTestClient(
169+
fn.WithRegistry(TestRegistry),
170+
fn.WithBuilder(builder),
171+
fn.WithPusher(pusher),
172+
))
173+
cmd.SetArgs([]string{})
174+
175+
if err := cmd.Execute(); err != nil {
176+
t.Fatalf("build %d failed: %v", i+1, err)
177+
}
178+
179+
// Load and verify after each build
180+
f, err := fn.NewFunction(root)
181+
if err != nil {
182+
t.Fatalf("loading function after build %d failed: %v", i+1, err)
183+
}
184+
185+
if !f.RegistryInsecure {
186+
t.Fatalf("build %d: registryInsecure should be true, but was false", i+1)
187+
}
188+
}
189+
})
190+
}
191+
192+
// TestWarnRegistryInsecureChange ensures that the warning is printed when
193+
// the registry changes but registryInsecure is still true.
194+
func TestWarnRegistryInsecureChange(t *testing.T) {
195+
tests := []struct {
196+
name string
197+
cfgRegistry string
198+
funcRegistry string
199+
funcInsecure bool
200+
expectWarning bool
201+
expectedMessage string
202+
}{
203+
{
204+
name: "no warning - registry not changed",
205+
cfgRegistry: "example.com/alice",
206+
funcRegistry: "example.com/alice",
207+
funcInsecure: true,
208+
expectWarning: false,
209+
},
210+
{
211+
name: "no warning - registryInsecure is false",
212+
cfgRegistry: "example.com/bob",
213+
funcRegistry: "example.com/alice",
214+
funcInsecure: false,
215+
expectWarning: false,
216+
},
217+
{
218+
name: "no warning - func registry is empty",
219+
cfgRegistry: "example.com/bob",
220+
funcRegistry: "",
221+
funcInsecure: true,
222+
expectWarning: false,
223+
},
224+
{
225+
name: "no warning - cfg registry is empty",
226+
cfgRegistry: "",
227+
funcRegistry: "example.com/alice",
228+
funcInsecure: true,
229+
expectWarning: false,
230+
},
231+
{
232+
name: "warning - registry changed and insecure is true",
233+
cfgRegistry: "example.com/bob",
234+
funcRegistry: "example.com/alice",
235+
funcInsecure: true,
236+
expectWarning: true,
237+
expectedMessage: "Warning: Registry changed from 'example.com/alice' to 'example.com/bob', but registryInsecure is still true.",
238+
},
239+
}
240+
241+
for _, tt := range tests {
242+
t.Run(tt.name, func(t *testing.T) {
243+
f := fn.Function{
244+
Registry: tt.funcRegistry,
245+
RegistryInsecure: tt.funcInsecure,
246+
}
247+
248+
// Capture output
249+
var buf bytes.Buffer
250+
warnRegistryInsecureChange(&buf, tt.cfgRegistry, f)
251+
252+
output := buf.String()
253+
254+
if tt.expectWarning {
255+
if output == "" {
256+
t.Fatal("expected warning but got none")
257+
}
258+
if tt.expectedMessage != "" && !strings.Contains(output, tt.expectedMessage) {
259+
t.Fatalf("expected message to contain '%s', got '%s'", tt.expectedMessage, output)
260+
}
261+
} else {
262+
if output != "" {
263+
t.Fatalf("expected no warning but got: %s", output)
264+
}
265+
}
266+
})
267+
}
268+
}

cmd/deploy.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ EXAMPLES
160160
fmt.Sprintf("Builder to use when creating the function's container. Currently supported builders are %s.", KnownBuilders()))
161161
cmd.Flags().StringP("registry", "r", cfg.Registry,
162162
"Container registry + registry namespace. (ex 'ghcr.io/myuser'). The full image name is automatically determined using this along with function name. ($FUNC_REGISTRY)")
163-
cmd.Flags().Bool("registry-insecure", cfg.RegistryInsecure, "Skip TLS certificate verification when communicating in HTTPS with the registry ($FUNC_REGISTRY_INSECURE)")
163+
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)")
164164
cmd.Flags().String("registry-authfile", "", "Path to a authentication file containing registry credentials ($FUNC_REGISTRY_AUTHFILE)")
165165

166166
// Function-Context Flags:
@@ -282,6 +282,10 @@ func runDeploy(cmd *cobra.Command, newClient ClientFactory) (err error) {
282282
if err = cfg.Validate(cmd); err != nil {
283283
return wrapValidateError(err, "deploy")
284284
}
285+
286+
// Warn if registry changed but registryInsecure is still true
287+
warnRegistryInsecureChange(cmd.OutOrStderr(), cfg.Registry, f)
288+
285289
if f, err = cfg.Configure(f); err != nil { // Updates f with deploy cfg
286290
return
287291
}

cmd/deploy_test.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2346,3 +2346,7 @@ func TestDeploy_ValidDomain(t *testing.T) {
23462346
})
23472347
}
23482348
}
2349+
2350+
func TestDeploy_RegistryInsecurePersists(t *testing.T) {
2351+
testRegistryInsecurePersists(NewDeployCmd, t)
2352+
}

docs/reference/func_build.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ func build
6969
-u, --push Attempt to push the function image to the configured registry after being successfully built
7070
-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)
7171
--registry-authfile string Path to a authentication file containing registry credentials ($FUNC_REGISTRY_AUTHFILE)
72-
--registry-insecure Skip TLS certificate verification when communicating in HTTPS with the registry ($FUNC_REGISTRY_INSECURE)
72+
--registry-insecure Skip TLS certificate verification when communicating in HTTPS with the registry. The value is persisted over consecutive runs ($FUNC_REGISTRY_INSECURE)
7373
-v, --verbose Print verbose logs ($FUNC_VERBOSE)
7474
```
7575

docs/reference/func_deploy.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ func deploy
134134
--pvc-size string When triggering a remote deployment, set a custom volume size to allocate for the build operation ($FUNC_PVC_SIZE)
135135
-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)
136136
--registry-authfile string Path to a authentication file containing registry credentials ($FUNC_REGISTRY_AUTHFILE)
137-
--registry-insecure Skip TLS certificate verification when communicating in HTTPS with the registry ($FUNC_REGISTRY_INSECURE)
137+
--registry-insecure Skip TLS certificate verification when communicating in HTTPS with the registry. The value is persisted over consecutive runs ($FUNC_REGISTRY_INSECURE)
138138
-R, --remote Trigger a remote deployment. Default is to deploy and build from the local system ($FUNC_REMOTE)
139139
--remote-storage-class string Specify a storage class to use for the volume on-cluster during remote builds
140140
--service-account string Service account to be used in the deployed function ($FUNC_SERVICE_ACCOUNT)

pkg/config/config.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,10 @@ func (c Global) Apply(f fn.Function) Global {
138138
if f.Registry != "" {
139139
c.Registry = f.Registry
140140
}
141+
// Unconditional because bool has no "empty value". Works because
142+
// viper resolves the correct precedence via our defaulting.
143+
c.RegistryInsecure = f.RegistryInsecure
144+
141145
return c
142146
}
143147

@@ -156,6 +160,10 @@ func (c Global) Configure(f fn.Function) fn.Function {
156160
if c.Registry != "" {
157161
f.Registry = c.Registry
158162
}
163+
// Unconditional because bool has no "empty value". Works because
164+
// viper resolves the correct precedence via our defaulting.
165+
f.RegistryInsecure = c.RegistryInsecure
166+
159167
return f
160168
}
161169

0 commit comments

Comments
 (0)