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
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ test-integration: ## Run integration tests using an available cluster.
.PHONY: test-e2e
test-e2e: func-instrumented-bin ## Basic E2E tests (includes core, metadata and remote tests)
# Runtime and other options can be configured using the FUNC_E2E_* environment variables. see e2e_test.go
go test -tags e2e -timeout 60m ./e2e -v -run "TestCore_|TestMetadata_|TestRemote_|TestLifecycle_"
go test -tags e2e -timeout 60m ./e2e -v -run "TestGateway_|TestCore_|TestMetadata_|TestRemote_|TestLifecycle_"
go tool covdata textfmt -i=$${FUNC_E2E_GOCOVERDIR:-.coverage} -o coverage.txt

.PHONY: test-e2e-podman
Expand Down
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", "gateway", "gateway:"}

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
}
70 changes: 66 additions & 4 deletions cmd/deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,10 +131,10 @@ EXAMPLES
SuggestFor: []string{"delpoy", "deplyo"},
PreRunE: bindEnv("build", "build-timestamp", "builder", "builder-image",
"base-image", "confirm", "domain", "env", "git-branch", "git-dir",
"git-url", "image", "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 @@ -199,6 +199,12 @@ EXAMPLES
"Image pull secret to use when the function's image is in a private registry ($FUNC_IMAGE_PULL_SECRET)")
cmd.Flags().String("deployer", f.Deploy.Deployer,
fmt.Sprintf("Type of deployment to use: '%s' for Knative Service (default), '%s' for Kubernetes Deployment or '%s' for Deployment with a Keda HTTP scaler ($FUNC_DEPLOY_TYPE)", knative.KnativeDeployerName, k8s.KubernetesDeployerName, keda.KedaDeployerName))
cmd.Flags().String("expose", f.Deploy.Expose,
"External exposure mode: 'gateway' (exposed, enforced, cluster-wide Gateway auto-discovery "+
"- the default), 'gateway:<ns>/<name>' (pin an exact Gateway), 'gateway:<ns>/' "+
"(restrict discovery to a namespace), 'none' (cluster-local opt-out). Raw deployer only. "+
"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 @@ -239,6 +245,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 @@ -560,6 +570,10 @@ 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 gateway (exposed).
Expose string
}

// newDeployConfig creates a buildConfig populated from command flags and
Expand All @@ -582,6 +596,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 @@ -619,6 +634,8 @@ func (c deployConfig) Configure(f fn.Function) (fn.Function, error) {
f.Deploy.ImagePullSecret = c.ImagePullSecret
f.Deploy.Deployer = c.Deployer
f.Deploy.ManagementDisabled = c.ManagementDisabled
// The effective value always lands on f; explicitly-empty flags are rejected in Validate.
f.Deploy.Expose = c.Expose
f.Local.Remote = c.Remote

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

// Validate expose format here (mirroring domain/namespace above) so a
// malformed value is rejected with CLI guidance before it ever reaches
// f.Write's Function.Validate, whose bundled error loses the sentinel.
if _, _, err = fn.ParseExpose(c.Expose); err != nil {
return err
}

// deploy.expose only takes effect for the raw deployer. For knative/keda
// a set value is ignored, so warn (fired here so it precedes any build
// work on both the local and remote deploy paths) and proceed - except
// where the deployer's fixed behavior already matches the user's intent
// (keda+none: keda is cluster-local already), which stays silent.
// Effective deployer is c.Deployer as configured; empty defaults to
// knative.
if c.Expose != "" {
effectiveDeployer := c.Deployer
if effectiveDeployer == "" {
effectiveDeployer = knative.KnativeDeployerName
}
switch effectiveDeployer {
case knative.KnativeDeployerName:
if c.Expose == "none" {
// Knative exposes externally by default regardless of this
// field, so "none" leaves the user's privacy intent unmet -
// unlike the other combos below, this warrants the stronger,
// privacy-specific wording naming the actual remedy.
fmt.Fprintln(cmd.OutOrStderr(), fmt.Sprintf(
"warning: deploy.expose %q is ignored by the knative deployer - "+
"Knative exposes this function externally by default; label the Service "+
"networking.knative.dev/visibility=cluster-local to keep it private.", c.Expose))
} else {
fmt.Fprintln(cmd.OutOrStderr(), fmt.Sprintf(
"warning: deploy.expose %q is ignored by the knative deployer - "+
"Knative manages exposure itself.", c.Expose))
}
case keda.KedaDeployerName:
if c.Expose != "none" {
fmt.Fprintln(cmd.OutOrStderr(), fmt.Sprintf(
"warning: deploy.expose %q is ignored by the keda deployer - "+
"external exposure for keda functions is not yet supported.", c.Expose))
}
// keda + none: silence - the user's intent is already met.
}
}

// Check Image Digest was included
var digest bool
if c.Image != "" {
Expand Down
212 changes: 212 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 @@ -2547,3 +2548,214 @@ func TestDeploy_ValidDomain(t *testing.T) {
func TestDeploy_RegistryInsecurePersists(t *testing.T) {
testRegistryInsecurePersists(NewDeployCmd, t)
}

// TestDeploy_ExposeExplicitEmptyClears: 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_ExposeExplicitEmptyClears(t *testing.T) {
// newFn initializes a Go function in a temp directory (the cwd for the
// deploys below) 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_ExposeGatewayRefPersists ensures the union "gateway:<ns>/<name>"
// form round-trips through --expose into f.Deploy.Expose end-to-end.
func TestDeploy_ExposeGatewayRefPersists(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=gateway:infra/gw"})
if err := cmd.Execute(); err != nil {
t.Fatal(err)
}

f, err := fn.NewFunction(root)
if err != nil {
t.Fatal(err)
}
if f.Deploy.Expose != "gateway:infra/gw" {
t.Fatalf("expected expose 'gateway:infra/gw' 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+gateway: silent",
args: []string{"--deployer", "raw", "--expose", "gateway"},
},
{
name: "knative+empty: silent",
args: []string{"--deployer", "knative"},
},
{
name: "knative+gateway: warns, proceeds",
args: []string{"--deployer", "knative", "--expose", "gateway"},
wantWarning: `deploy.expose "gateway" is ignored by the knative deployer`,
},
{
name: "knative+none: privacy-specific warning, proceeds",
args: []string{"--deployer", "knative", "--expose", "none"},
wantWarning: `networking.knative.dev/visibility=cluster-local`,
},
{
name: "keda+gateway: warns, proceeds",
args: []string{"--deployer", "keda", "--expose", "gateway"},
wantWarning: `deploy.expose "gateway" is ignored by the keda deployer`,
},
{
name: "keda+none: silent (intent already met, keda is cluster-local)",
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())
}
})
}
}
Loading
Loading