env opaque box test

client_int_test.go

@@ -20,16 +20,20 @@ import (
 
 /*
  NOTE: Running integration tests locally requires a configured test cluster.
- Test failures may require manual removal of dangling resources.
+ Test failures may require manual removal of dangling resources, though
+ a complete rebuild and destruction of the integration cluster is
+ suggested.
 
- ## Integration Cluster
+ ## Creating an Integration-Test Cluster
 
  These integration tests require a properly configured cluster,
  such as that which is setup and configured in CI (see .github/workflows).
  A local KinD cluster can be started via:
  ./hack/allocate.sh && ./hack/configure.sh
+ Note that kubectl, kind and yq are required. on linux they can be installed
+ with ./hack/binaries.sh
 
- ## Integration Testing
+ ## Running Integration Tests
 
  These tests can be run via the make target:
  make test-integration
@@ -38,10 +42,11 @@ import (
 
  ## Teardown and Cleanup
 
- Tests should clean up after themselves. In the event of failures, one may
- need to manually remove files:
- rm -rf ./testdata/example.com
- The test cluster is not automatically removed, as it can be reused. To remove:
+ Tests should clean up after themselves, including cluster resources and
+ temp directoreis. However some tests have not yet been updated, so failed
+ tests may leave dangling files in ./testdata which must be manually removed.
+ Additinally, failed tests may not clean up all their cluster resources, so
+ removing the test cluster after a test run ends in failure is suggested:
  ./hack/delete.sh
 */
 

cmd/build.go

@@ -151,6 +151,8 @@ func runBuild(cmd *cobra.Command, _ []string, newClient ClientFactory) (err erro
  }
  f = cfg.Configure(f) // Updates f with build cfg
 
+ // TODO: this logic is duplicated with runDeploy. Shouild be in buildConfig
+ // constructor.
  // Checks if there is a difference between defined registry and its value used as a prefix in the image tag
  // In case of a mismatch a new image tag is created and used for build
  // Do not react if image tag has been changed outside configuration

cmd/deploy.go

@@ -534,29 +534,34 @@ func newDeployConfig(cmd *cobra.Command) (c deployConfig) {
 // current values, as they were passed through via flag defaults.
 func (c deployConfig) Configure(f fn.Function) (fn.Function, error) {
  var err error
+
+ // Bubble configure request
+ //
  // The member values on the config object now take absolute precidence
  // because they include 1) static config 2) user's global config
  // 3) Environment variables and 4) flag values (which were set with their
  // default being 1-3).
  f = c.buildConfig.Configure(f) // also configures .buildConfig.Global
- // deploy adds the ability to specify a digest on the associated image
- f.ImageDigest, err = imageDigest(f.Image)
- if err != nil {
- return f, err
- }
- // f.Build is not necessary
+
+ // Configure basic members
  f.Build.Git.URL = c.GitURL
  f.Build.Git.ContextDir = c.GitDir
- f.Build.Git.Revision = c.GitBranch // TODO: shouild match; perhaps be "refSpec"
+ f.Build.Git.Revision = c.GitBranch // TODO: shouild match; perhaps "refSpec"
  f.Deploy.Namespace = c.Namespace
  f.Deploy.Remote = c.Remote
 
- // TODO: validate env test cases completely validate:
- envsToUpdate, envsToRemove, err := util.OrderedMapAndRemovalListFromArray(c.Env, "=")
+ // ImageDigest
+ // Parsed off f.Image if provided. Deploying adds the ability to specify a
+ // digest on the associated image (not available on build as nonsensical).
+ f.ImageDigest, err = imageDigest(f.Image)
  if err != nil {
  return f, err
  }
- f.Run.Envs, _, err = mergeEnvs(f.Run.Envs, envsToUpdate, envsToRemove)
+
+ // Envs
+ // Preprocesses any Envs provided (which may include removals) into a final
+ // set
+ f.Run.Envs, err = applyEnvs(f.Run.Envs, c.Env)
  if err != nil {
  return f, err
  }
@@ -572,6 +577,20 @@ func (c deployConfig) Configure(f fn.Function) (fn.Function, error) {
  return f, nil
 }
 
+// Apply Env additions/removals to a set of extant envs, returning the final
+// merged list.
+func applyEnvs(current []fn.Env, args []string) (final []fn.Env, err error) {
+ // TODO: validate env test cases completely validate this functionality
+
+ // Parse and Merge
+ inserts, removals, err := util.OrderedMapAndRemovalListFromArray(args, "=")
+ if err != nil {
+ return
+ }
+ final, _, err = mergeEnvs(current, inserts, removals)
+ return
+}
+
 // Prompt the user with value of config members, allowing for interaractive changes.
 // Skipped if not in an interactive terminal (non-TTY), or if --yes (agree to
 // all prompts) was explicitly set.
@@ -620,7 +639,7 @@ func (c deployConfig) Prompt() (deployConfig, error) {
  }
  }
 
- // TODO(lkingland) prompt for optional additional git settings
+ // TODO: prompt for optional additional git settings here:
  // if c.GitURL != "" {
  // }
 
@@ -674,16 +693,15 @@ func (c deployConfig) Validate() (err error) {
  return fmt.Errorf("invalid --git-url '%v'", c.GitURL)
  }
 
- // NOTE: (--registry) no explicit check for --registry or --image here, because
- // this logic is being baked into the core, and it will validate the cases
- // and return fn.ErrNameRequired, fn.ErrImageRequired etc as needed.
+ // NOTE: There is no explicit check for --registry or --image here, because
+ // this logic is baked into core, which will validate the cases and return
+ // an fn.ErrNameRequired, fn.ErrImageRequired etc as needed.
 
  return
 }
 
-// imageDigest splits out and returns the image digest from a full image
-// string if it is included, and an error if an image digest is provided but
-// is in an invalid format.
+// imageDigest returns the image digest from a full image string if it exists,
+// and includes basic validation that a provided digest is correctly formatted.
 func imageDigest(v string) (digest string, err error) {
  vv := strings.Split(v, "@")
  if len(vv) < 2 {

cmd/deploy_test.go

@@ -5,6 +5,7 @@ import (
  "errors"
  "fmt"
  "path/filepath"
+ "reflect"
  "strings"
  "testing"
  "time"
@@ -1174,14 +1175,12 @@ func TestDeploy_NamespaceRedeployWarning(t *testing.T) {
 // TestDeploy_RemotePersists ensures that the remote field is read from
 // the function by default, and is able to be overridden by flags/env vars.
 func TestDeploy_RemotePersists(t *testing.T) {
- t.Helper()
  root := fromTempDirectory(t)
- cmdFn := NewDeployCmd
 
  if err := fn.New().Create(fn.Function{Runtime: "node", Root: root}); err != nil {
  t.Fatal(err)
  }
- cmd := cmdFn(NewTestClient(fn.WithRegistry(TestRegistry)))
+ cmd := NewDeployCmd(NewTestClient(fn.WithRegistry(TestRegistry)))
  cmd.SetArgs([]string{})
  if err := cmd.Execute(); err != nil {
  t.Fatal(err)
@@ -1191,7 +1190,7 @@ func TestDeploy_RemotePersists(t *testing.T) {
  var f fn.Function
 
  // Deploy the function, specifying remote deployment(on-cluster)
- cmd = cmdFn(NewTestClient(fn.WithRegistry(TestRegistry)))
+ cmd = NewDeployCmd(NewTestClient(fn.WithRegistry(TestRegistry)))
  cmd.SetArgs([]string{"--remote"})
  if err := cmd.Execute(); err != nil {
  t.Fatal(err)
@@ -1205,7 +1204,7 @@ func TestDeploy_RemotePersists(t *testing.T) {
  }
 
  // Deploy the function again without specifying remote
- cmd = cmdFn(NewTestClient(fn.WithRegistry(TestRegistry)))
+ cmd = NewDeployCmd(NewTestClient(fn.WithRegistry(TestRegistry)))
  cmd.SetArgs([]string{})
  if err := cmd.Execute(); err != nil {
  t.Fatal(err)
@@ -1235,3 +1234,92 @@ func TestDeploy_RemotePersists(t *testing.T) {
  t.Fatalf("value of remote flag not persisted")
  }
 }
+
+// TestDeploy_Envs ensures that environment variable for the function, provided
+// as arguments, are correctly evaluated. This includes:
+// - Multiple Envs are supported (flag can be provided multiple times)
+// - Existing Envs on the function are retained
+// - Flags provided with the minus '-' suffix are treated as a removal
+func TestDeploy_Envs(t *testing.T) {
+ var (
+ root = fromTempDirectory(t)
+ ptr = func(s string) *string { return &s } // TODO: remove pointers from Envs.
+ f fn.Function
+ cmd *cobra.Command
+ err error
+ expected []fn.Env
+ )
+
+ if err = fn.New().Create(fn.Function{Runtime: "go", Root: root, Registry: TestRegistry}); err != nil {
+ t.Fatal(err)
+ }
+
+ // Deploy the function with two environment variables specified.
+ cmd = NewDeployCmd(NewTestClient())
+ cmd.SetArgs([]string{"--env=ENV1=VAL1", "--env=ENV2=VAL2"})
+ if err = cmd.Execute(); err != nil {
+ t.Fatal(err)
+ }
+ // Assert it contains the two environment variables
+ if f, err = fn.NewFunction(root); err != nil {
+ t.Fatal(err)
+ }
+ expected = []fn.Env{
+ {ptr("ENV1"), ptr("VAL1")},
+ {ptr("ENV2"), ptr("VAL2")},
+ }
+ if !reflect.DeepEqual(f.Run.Envs, expected) {
+ t.Fatalf("Expected envs '%v', got '%v'", expected, f.Run.Envs)
+ }
+
+ // Deploy the function with an additinal environment variable.
+ cmd = NewDeployCmd(NewTestClient())
+ cmd.SetArgs([]string{"--env=ENV3=VAL3"})
+ if err = cmd.Execute(); err != nil {
+ t.Fatal(err)
+ }
+ // Assert the original envs were retained and the new env was added.
+ if f, err = fn.NewFunction(root); err != nil {
+ t.Fatal(err)
+ }
+ expected = []fn.Env{
+ {ptr("ENV1"), ptr("VAL1")},
+ {ptr("ENV2"), ptr("VAL2")},
+ {ptr("ENV3"), ptr("VAL3")},
+ }
+ if !reflect.DeepEqual(f.Run.Envs, expected) {
+ t.Fatalf("Expected envs '%v', got '%v'", expected, f.Run.Envs)
+ }
+
+ // Deploy the function with a removal instruction
+ cmd = NewDeployCmd(NewTestClient())
+ cmd.SetArgs([]string{"--env=ENV1-"})
+ if err := cmd.Execute(); err != nil {
+ t.Fatal(err)
+ }
+ if f, err = fn.NewFunction(root); err != nil {
+ t.Fatal(err)
+ }
+ expected = []fn.Env{
+ {ptr("ENV2"), ptr("VAL2")},
+ {ptr("ENV3"), ptr("VAL3")},
+ }
+ if !reflect.DeepEqual(f.Run.Envs, expected) {
+ t.Fatalf("Expected envs '%v', got '%v'", expected, f.Run.Envs)
+ }
+
+ // Try removing the rest for good measure
+ cmd = NewDeployCmd(NewTestClient())
+ cmd.SetArgs([]string{"--env=ENV2-", "--env=ENV3-"})
+ if err := cmd.Execute(); err != nil {
+ t.Fatal(err)
+ }
+ if f, err = fn.NewFunction(root); err != nil {
+ t.Fatal(err)
+ }
+ if len(f.Run.Envs) != 0 {
+ t.Fatalf("Expected no envs to remain, got '%v'", f.Run.Envs)
+ }
+
+ // TODO: create and test typed errors for ErrEnvNotExist etc.
+}