Skip to content

Commit d4f2a40

Browse files
creydrgauron99
andauthored
[release-1.20] Remember --registry-insecure setting on recurring runs + backport fixes (#3588)
* [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> * Run update-codegen.sh * Fix linter issues * Pin cloudevents to 1.x * Use component-versions from 1.20 * smarter component updates (#3290) * Replace tekton hub with direct bundle references * Fix TestGitlab by restoring valid Go application code The test was creating a broken Procfile instead of a working main.go, causing the pipeline build to fail and the test to timeout. * Revert "Fix TestGitlab by restoring valid Go application code" This reverts commit 3f39a10. * Replace tekton hub with direct bundle references (2) --------- Co-authored-by: David Fridrich <49119790+gauron99@users.noreply.github.com>
1 parent 7052943 commit d4f2a40

File tree

17 files changed

+7886
-7216
lines changed

17 files changed

+7886
-7216
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"
@@ -100,7 +102,7 @@ EXAMPLES
100102
fmt.Sprintf("Builder to use when creating the function's container. Currently supported builders are %s. ($FUNC_BUILDER)", KnownBuilders()))
101103
cmd.Flags().StringP("registry", "r", cfg.Registry,
102104
"Container registry + registry namespace. (ex 'ghcr.io/myuser'). The full image name is automatically determined using this along with function name. ($FUNC_REGISTRY)")
103-
cmd.Flags().Bool("registry-insecure", cfg.RegistryInsecure, "Skip TLS certificate verification when communicating in HTTPS with the registry ($FUNC_REGISTRY_INSECURE)")
105+
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)")
104106

105107
// Function-Context Flags:
106108
// Options whose value is available on the function with context only
@@ -209,6 +211,10 @@ For more options, run 'func build --help'`, err)
209211
if !f.Initialized() {
210212
return fn.NewErrNotInitialized(f.Root)
211213
}
214+
215+
// Warn if registry changed but registryInsecure is still true
216+
warnRegistryInsecureChange(os.Stderr, cfg.Registry, f)
217+
212218
f = cfg.Configure(f) // Returns an f updated with values from the config (flags, envs, etc)
213219

214220
// Client
@@ -240,6 +246,15 @@ For more options, run 'func build --help'`, err)
240246
return f.Stamp()
241247
}
242248

249+
// warnRegistryInsecureChange checks if the registry has changed but
250+
// registryInsecure is still set to true, and prints a warning if so.
251+
// This helps users avoid accidentally skipping TLS verification on a new registry.
252+
func warnRegistryInsecureChange(w io.Writer, newRegistry string, f fn.Function) {
253+
if f.Registry != "" && newRegistry != "" && f.Registry != newRegistry && f.RegistryInsecure {
254+
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)
255+
}
256+
}
257+
243258
type buildConfig struct {
244259
// Globals (builder, confirm, registry, verbose)
245260
config.Global
@@ -432,7 +447,10 @@ func (c buildConfig) Validate(cmd *cobra.Command) (err error) {
432447
// image necessary for the target cluster, since the end product of a function
433448
// deployment is not the contiainer, but rather the running service.
434449
func (c buildConfig) clientOptions() ([]fn.Option, error) {
435-
o := []fn.Option{fn.WithRegistry(c.Registry)}
450+
o := []fn.Option{
451+
fn.WithRegistry(c.Registry),
452+
fn.WithRegistryInsecure(c.RegistryInsecure),
453+
}
436454
switch c.Builder {
437455
case builders.Host:
438456
t := newTransport(c.RegistryInsecure) // may provide a custom impl which proxies
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
@@ -159,7 +159,7 @@ EXAMPLES
159159
fmt.Sprintf("Builder to use when creating the function's container. Currently supported builders are %s.", KnownBuilders()))
160160
cmd.Flags().StringP("registry", "r", cfg.Registry,
161161
"Container registry + registry namespace. (ex 'ghcr.io/myuser'). The full image name is automatically determined using this along with function name. ($FUNC_REGISTRY)")
162-
cmd.Flags().Bool("registry-insecure", cfg.RegistryInsecure, "Skip TLS certificate verification when communicating in HTTPS with the registry ($FUNC_REGISTRY_INSECURE)")
162+
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)")
163163

164164
// Function-Context Flags:
165165
// Options whose value is available on the function with context only
@@ -310,6 +310,10 @@ For more options, run 'func deploy --help'`, err)
310310
}
311311
return
312312
}
313+
314+
// Warn if registry changed but registryInsecure is still true
315+
warnRegistryInsecureChange(cmd.OutOrStderr(), cfg.Registry, f)
316+
313317
if f, err = cfg.Configure(f); err != nil { // Updates f with deploy cfg
314318
return
315319
}

cmd/deploy_test.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2190,3 +2190,7 @@ func testBaseImage(cmdFn commandConstructor, t *testing.T) {
21902190
})
21912191
}
21922192
}
2193+
2194+
func TestDeploy_RegistryInsecurePersists(t *testing.T) {
2195+
testRegistryInsecurePersists(NewDeployCmd, t)
2196+
}

docs/reference/func_build.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ func build
6868
--platform string Optionally specify a target platform, for example "linux/amd64" when using the s2i build strategy
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)
71-
--registry-insecure Skip TLS certificate verification when communicating in HTTPS with the registry ($FUNC_REGISTRY_INSECURE)
71+
--registry-insecure Skip TLS certificate verification when communicating in HTTPS with the registry. The value is persisted over consecutive runs ($FUNC_REGISTRY_INSECURE)
7272
-v, --verbose Print verbose logs ($FUNC_VERBOSE)
7373
```
7474

docs/reference/func_deploy.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ func deploy
132132
-u, --push Push the function image to registry before deploying. ($FUNC_PUSH) (default true)
133133
--pvc-size string When triggering a remote deployment, set a custom volume size to allocate for the build operation ($FUNC_PVC_SIZE)
134134
-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)
135-
--registry-insecure Skip TLS certificate verification when communicating in HTTPS with the registry ($FUNC_REGISTRY_INSECURE)
135+
--registry-insecure Skip TLS certificate verification when communicating in HTTPS with the registry. The value is persisted over consecutive runs ($FUNC_REGISTRY_INSECURE)
136136
-R, --remote Trigger a remote deployment. Default is to deploy and build from the local system ($FUNC_REMOTE)
137137
--remote-storage-class string Specify a storage class to use for the volume on-cluster during remote builds
138138
--service-account string Service account to be used in the deployed function ($FUNC_SERVICE_ACCOUNT)

0 commit comments

Comments
 (0)