Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions cmd/completion_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,3 +190,23 @@ func CompleteDeployerList(cmd *cobra.Command, args []string, complete string) (m

return
}

func CompleteExposeList(cmd *cobra.Command, args []string, complete string) (matches []string, d cobra.ShellCompDirective) {
values := []string{"none", "route"}

d = cobra.ShellCompDirectiveNoFileComp
matches = []string{}

if len(complete) == 0 {
matches = values
return
}

for _, v := range values {
if strings.HasPrefix(v, complete) {
matches = append(matches, v)
}
}

return
}
47 changes: 43 additions & 4 deletions cmd/deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,10 +132,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", "image-pull-secret", "management-disabled", "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", "management-disabled",
"namespace", "path", "platform", "push", "pvc-size", "service-account",
"deployer", "expose", "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)
},
Expand Down Expand Up @@ -200,6 +200,12 @@ EXAMPLES
"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("expose", f.Deploy.Expose,
"External exposure mode: 'route' (create a Route; OpenShift clusters only), "+
"'none' (cluster-local opt-out). Raw and keda deployers only. "+
"Defaults to exposed on OpenShift, cluster-local elsewhere. "+
"An explicitly empty value (--expose=\"\") clears the persisted deploy.expose key and "+
"returns to the default. ($FUNC_EXPOSE)")
// Static Flags:
// Options which have static defaults only (not globally configurable nor
// persisted with the function)
Expand Down Expand Up @@ -240,6 +246,10 @@ EXAMPLES
fmt.Println("internal: error while calling RegisterFlagCompletionFunc: ", err)
}

if err := cmd.RegisterFlagCompletionFunc("expose", CompleteExposeList); err != nil {
fmt.Println("internal: error while calling RegisterFlagCompletionFunc: ", err)
}

return cmd
}

Expand Down Expand Up @@ -285,6 +295,10 @@ func runDeploy(cmd *cobra.Command, newClient ClientFactory) (err error) {
// Warn if registry changed but registryInsecure is still true
warnRegistryInsecureChange(cmd.OutOrStderr(), cfg.Registry, f)

// Warn if deploy.expose is set (by flag or persisted in func.yaml) for a
// deployer that ignores it
warnExposeIgnore(cmd.OutOrStderr(), cfg.Expose, cfg.Deployer)

// Back-compat: a function deployed before the deployer was recorded has a
// namespace but no deployer, which historically could only mean knative.
if f.Deploy.Namespace != "" && f.Deploy.Deployer == "" {
Expand Down Expand Up @@ -570,6 +584,11 @@ type deployConfig struct {

// ManagementDisabled disables automatic Function CR sync after deploy.
ManagementDisabled bool

// Expose controls external access - how/if the function should be
// exposed externally. Defaults to exposed on OpenShift, cluster-local
// elsewhere; "none" opts out explicitly.
Expose string
}

// newDeployConfig creates a buildConfig populated from command flags and
Expand All @@ -592,6 +611,7 @@ func newDeployConfig(cmd *cobra.Command) deployConfig {
ImagePullSecret: viper.GetString("image-pull-secret"),
Deployer: viper.GetString("deployer"),
ManagementDisabled: viper.GetBool("management-disabled"),
Expose: viper.GetString("expose"),
}
// NOTE: .Env should be viper.GetStringSlice, but this returns unparsed
// results and appears to be an open issue since 2017:
Expand Down Expand Up @@ -629,6 +649,7 @@ func (c deployConfig) Configure(f fn.Function) (fn.Function, error) {
f.Deploy.ImagePullSecret = c.ImagePullSecret
f.Deployer = c.Deployer
f.Deploy.ManagementDisabled = c.ManagementDisabled
f.Deploy.Expose = c.Expose
f.Local.Remote = c.Remote

// PVCSize
Expand Down Expand Up @@ -748,6 +769,11 @@ func (c deployConfig) Validate(cmd *cobra.Command) (err error) {
}
}

// Validate expose flag if provided
if err = fn.ValidateExpose(c.Expose); err != nil {
return err
}

// Check Image Digest was included
var digest bool
if c.Image != "" {
Expand Down Expand Up @@ -909,3 +935,16 @@ func isDigested(v string) (validDigest bool, err error) {
_, ok := ref.(name.Digest)
return ok, nil
}

// warnExposeIgnore warns when a non-empty deploy.expose is paired with a
// deployer that ignores it. The value is the RESOLVED one, not just what the
// user typed: the --expose flag registers f.Deploy.Expose as its own default,
// so a value persisted in func.yaml warns on its own with no flag present.
// An empty deployer means the default (knative), which also ignores expose.
func warnExposeIgnore(w io.Writer, expose, deployer string) {
if expose != "" && deployer != k8s.KubernetesDeployerName &&
deployer != keda.KedaDeployerName {
fmt.Fprintf(w, "warning: deploy.expose %q is ignored - only the raw and keda deployers "+
"support external exposure via this field.\n", expose)
}
}
210 changes: 210 additions & 0 deletions cmd/deploy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"errors"
"fmt"
"os"
"path/filepath"
"reflect"
"strings"
Expand Down Expand Up @@ -2729,3 +2730,212 @@ func TestDeploy_DeployerSwitch(t *testing.T) {
})
}
}

// TestDeploy_ExposeEmptyVsUnset: an explicitly empty --expose=""
// clears the persisted deploy.expose key reverting to the default at deploy
// time, while a deploy without the flag leaves the persisted value untouched.
func TestDeploy_ExposeEmptyVsUnset(t *testing.T) {
// newFn initializes a Go function in a temp directory and returns its root.
newFn := func(t *testing.T) string {
t.Helper()
root := FromTempDirectory(t)
if _, err := fn.New().Init(fn.Function{Runtime: "go", Root: root}); err != nil {
t.Fatal(err)
}
return root
}

// deploy runs `func deploy` with args against mock builder/deployer,
// failing the test on error and returning the command's combined output.
deploy := func(t *testing.T, args ...string) string {
t.Helper()
cmd := NewDeployCmd(NewTestClient(
fn.WithBuilder(mock.NewBuilder()),
fn.WithDeployer(mock.NewDeployer()),
fn.WithRegistry(TestRegistry),
))
cmd.SetArgs(args)
var out strings.Builder
cmd.SetOut(&out)
cmd.SetErr(&out)
if err := cmd.Execute(); err != nil {
t.Fatal(err)
}
return out.String()
}

// loadFn re-reads the function from disk.
loadFn := func(t *testing.T, root string) fn.Function {
t.Helper()
f, err := fn.NewFunction(root)
if err != nil {
t.Fatal(err)
}
return f
}

t.Run(`--expose="" clears a previously-persisted "none"`, func(t *testing.T) {
root := newFn(t)

deploy(t, "--deployer", "raw", "--expose", "none")
if f := loadFn(t, root); f.Deploy.Expose != "none" {
t.Fatalf("setup: expected expose 'none' to be persisted, got %q", f.Deploy.Expose)
}

deploy(t, "--deployer", "raw", "--expose=")
// unmarshalled yaml would not be able to distinguish between the value
// being empty and gone (not in the file)
raw, err := os.ReadFile(filepath.Join(root, "func.yaml"))
if err != nil {
t.Fatal(err)
}
if strings.Contains(string(raw), "expose") {
t.Errorf("expected NO expose key in func.yaml, got:\n%s", raw)
}
})

t.Run("plain deploy without the flag still works and leaves expose unpersisted", func(t *testing.T) {
root := newFn(t)
deploy(t, "--deployer", "raw")
if f := loadFn(t, root); f.Deploy.Expose != "" {
t.Errorf("expected expose to remain unpersisted (empty), got %q", f.Deploy.Expose)
}
})

t.Run("persisted none + no flag round-trips untouched", func(t *testing.T) {
root := newFn(t)

deploy(t, "--deployer", "raw", "--expose", "none")
if f := loadFn(t, root); f.Deploy.Expose != "none" {
t.Fatalf("expected expose 'none' to be persisted, got %q", f.Deploy.Expose)
}

// redeploy without changing the flag should keep it as is
deploy(t, "--deployer", "raw")
if f := loadFn(t, root); f.Deploy.Expose != "none" {
t.Errorf("expected persisted 'none' to round-trip untouched, got %q", f.Deploy.Expose)
}
})
}

// TestDeploy_ExposeInvalidValueError: a malformed --expose value fails the
// deploy (any deployer) with the CLI's typed ErrInvalidExpose.
func TestDeploy_ExposeInvalidValueError(t *testing.T) {
root := FromTempDirectory(t)
if _, err := fn.New().Init(fn.Function{Runtime: "go", Root: root}); err != nil {
t.Fatal(err)
}
cmd := NewDeployCmd(NewTestClient(
fn.WithBuilder(mock.NewBuilder()),
fn.WithDeployer(mock.NewDeployer()),
fn.WithRegistry(TestRegistry),
))
cmd.SetArgs([]string{"--expose", "bogus"})
var want *ErrInvalidExpose
if err := cmd.Execute(); !errors.As(err, &want) {
t.Errorf("expected ErrInvalidExpose, got %v", err)
}
}

// TestDeploy_ExposeRoutePersists ensures "route" round-trips through
// --expose into f.Deploy.Expose end-to-end.
func TestDeploy_ExposeRoutePersists(t *testing.T) {
root := FromTempDirectory(t)

if _, err := fn.New().Init(fn.Function{Runtime: "go", Root: root}); err != nil {
t.Fatal(err)
}

cmd := NewDeployCmd(NewTestClient(
fn.WithBuilder(mock.NewBuilder()),
fn.WithDeployer(mock.NewDeployer()),
fn.WithRegistry(TestRegistry),
))
cmd.SetArgs([]string{"--deployer", "raw", "--expose=route"})
if err := cmd.Execute(); err != nil {
t.Fatal(err)
}

f, err := fn.NewFunction(root)
if err != nil {
t.Fatal(err)
}
if f.Deploy.Expose != "route" {
t.Fatalf("expected expose 'route' to be persisted, got %q", f.Deploy.Expose)
}
}

// TestDeploy_ExposeIgnoredByDeployerNote: a deployer that ignores a set
// deploy.expose warns and proceeds
func TestDeploy_ExposeIgnoredByDeployerNote(t *testing.T) {
tests := []struct {
name string
args []string
wantWarning string // distinguishing substring of the warning; "" means silent
}{
{
name: "raw+route: silent",
args: []string{"--deployer", "raw", "--expose", "route"},
},
{
name: "knative+empty: silent",
args: []string{"--deployer", "knative"},
},
{
name: "knative+route: warns, proceeds",
args: []string{"--deployer", "knative", "--expose", "route"},
wantWarning: `deploy.expose "route" is ignored - only the raw and keda deployers support external exposure via this field.`,
},
{
name: "knative+none: warns, proceeds",
args: []string{"--deployer", "knative", "--expose", "none"},
wantWarning: `deploy.expose "none" is ignored - only the raw and keda deployers support external exposure via this field.`,
},
{
name: "keda+route: silent, keda supports expose too",
args: []string{"--deployer", "keda", "--expose", "route"},
},
{
name: "keda+none: silent, keda supports expose too",
args: []string{"--deployer", "keda", "--expose", "none"},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
root := FromTempDirectory(t)
if _, err := fn.New().Init(fn.Function{Runtime: "go", Root: root}); err != nil {
t.Fatal(err)
}

builder := mock.NewBuilder()
cmd := NewDeployCmd(NewTestClient(
fn.WithBuilder(builder),
fn.WithDeployer(mock.NewDeployer()),
fn.WithRegistry(TestRegistry),
))
cmd.SetArgs(tt.args)
var stderr strings.Builder
cmd.SetOut(&stderr)
cmd.SetErr(&stderr)
err := cmd.Execute()

if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if !builder.BuildInvoked {
t.Error("expected the deploy to proceed to build")
}

if tt.wantWarning == "" {
if strings.Contains(stderr.String(), "deploy.expose") {
t.Errorf("expected no warning on stderr, got:\n%s", stderr.String())
}
return
}
if !strings.Contains(stderr.String(), tt.wantWarning) {
t.Errorf("expected stderr to contain:\n%s\ngot:\n%s", tt.wantWarning, stderr.String())
}
})
}
}
29 changes: 29 additions & 0 deletions cmd/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ Internal error during error-wrapping: specified cmd '%s' not supported`, cmd)
if errors.Is(err, fn.ErrPlatformNotSupported) {
return NewErrPlatformNotSupported(err, cmd)
}
if errors.Is(err, fn.ErrInvalidExpose) {
return NewErrInvalidExpose(err)
}
return err
}

Expand Down Expand Up @@ -217,6 +220,32 @@ func (e *ErrInvalidDomain) Unwrap() error {

// -------------------------------------------------------------------------- //

type ErrInvalidExpose struct {
Err error
}

func NewErrInvalidExpose(err error) error {
return &ErrInvalidExpose{Err: err}
}

func (e *ErrInvalidExpose) Error() string {
return fmt.Sprintf(`%v

Try this:
func deploy --expose=route Create an OpenShift Route (OpenShift clusters only)
func deploy --expose=none Cluster-local opt-out, no external exposure

deploy.expose takes effect with the raw and keda deployers only (--deployer=raw or --deployer=keda),
which expose by default when the platform and deployer support it.
For more options, run 'func deploy --help'`, e.Err)
}

func (e *ErrInvalidExpose) Unwrap() error {
return e.Err
}

// -------------------------------------------------------------------------- //

type ErrInvalidKubeconfig struct {
Err error
}
Expand Down
Loading
Loading