update tekton namespace handling

cmd/deploy.go

@@ -263,7 +263,7 @@ func runDeploy(cmd *cobra.Command, newClient ClientFactory) (err error) {
 	// If we're changing namespace in an OpenShift cluster, we have to
 	// also update the registry because there is a registry per namespace,
 	// and their name includes the namespace.
-	// This saves needing a manual flag ``--registyry={destination namespace registry}``
+	// This saves needing a manual flag ``--registry={destination namespace registry}``
 	if changingNamespace(f) && k8s.IsOpenShift() {
 		// TODO(lkingland): this appears to force use of the openshift
 		// internal registry.

cmd/deploy_test.go

@@ -571,7 +571,7 @@ func TestDeploy_GitArgsUsed(t *testing.T) {
 
 	// A Pipelines Provider which will validate the expected values were received
 	pipeliner := mock.NewPipelinesProvider()
-	pipeliner.RunFn = func(f fn.Function) (string, string, error) {
+	pipeliner.RunFn = func(f fn.Function) (string, fn.Function, error) {
 		if f.Build.Git.URL != url {
 			t.Errorf("Pipeline Provider expected git URL '%v' got '%v'", url, f.Build.Git.URL)
 		}
@@ -581,7 +581,7 @@ func TestDeploy_GitArgsUsed(t *testing.T) {
 		if f.Build.Git.ContextDir != dir {
 			t.Errorf("Pipeline Provider expected git dir '%v' got '%v'", url, f.Build.Git.ContextDir)
 		}
-		return url, "", nil
+		return url, f, nil
 	}
 
 	// Deploy the Function specifying all of the git-related flags and --remote
@@ -1860,7 +1860,6 @@ func TestDeploy_NoErrorOnOldFunctionNotFound(t *testing.T) {
 	clientFn := NewTestClient(
 		fn.WithDeployer(mock.NewDeployer()),
 		fn.WithRemover(remover),
-		fn.WithPipelinesProvider(mock.NewPipelinesProvider()),
 	)
 
 	// Create a basic go Function

cmd/root_test.go

@@ -324,20 +324,6 @@ func Test_defaultNamespace(t *testing.T) {
 
 		})
 	}
-
-	// t.Setenv("KUBECONFIG", filepath.Join(t.TempDir(), "nonexistent"))
-	// t.Setenv("KUBERNETES_SERVICE_HOST", "")
-	// t.Setenv("XDG_CONFIG_HOME", home)
-	// if config.DefaultNamespace() != "default" {
-	// 	t.Fatalf("did not receive expected default namespace 'default', got '%v'", config.DefaultNamespace())
-	// }
-	//
-	// // should be "func" when active k8s namespace is "func"
-	// kubeconfig := filepath.Join(cwd, "testdata", "TestDefaultNamespace", "kubeconfig")
-	// t.Setenv("KUBECONFIG", kubeconfig)
-	// if config.DefaultNamespace() != "func" {
-	// 	t.Fatalf("expected default namespace of 'func' when that is the active k8s namespace.  Got '%v'", config.DefaultNamespace())
-	// }
 }
 
 // Helpers

pkg/config/config_test.go

@@ -221,9 +221,6 @@ func TestApply(t *testing.T) {
 	if cfg.Language == "" {
 		t.Error("empty f.Runtime should not be mapped")
 	}
-	// if cfg.Namespace == "" {
-	// 	t.Error("empty f.Namespace should not be mapped")
-	// }
 	if cfg.Registry == "" {
 		t.Error("empty f.Registry should not be mapped")
 	}

pkg/functions/client.go

@@ -198,7 +198,7 @@ type DNSProvider interface {
 
 // PipelinesProvider manages lifecyle of CI/CD pipelines used by a function
 type PipelinesProvider interface {
-	Run(context.Context, Function) (string, string, error)
+	Run(context.Context, Function) (string, Function, error)
 	Remove(context.Context, Function) error
 	ConfigurePAC(context.Context, Function, any) error
 	RemovePAC(context.Context, Function, any) error
@@ -814,31 +814,13 @@ func (c *Client) Deploy(ctx context.Context, f Function, oo ...DeployOption) (Fu
 // Returned function contains applicable registry and deployed image name.
 // String is the default route.
 func (c *Client) RunPipeline(ctx context.Context, f Function) (string, Function, error) {
-	var err error
-	var url string
 	// Default function registry to the client's global registry
 	if f.Registry == "" {
 		f.Registry = c.registry
 	}
 
-	// If no image name has been specified by user (--image), calculate.
-	// Image name is stored on the function for later use by deploy.
-	if f.Image != "" {
-		// if user specified an image, use it
-		f.Deploy.Image = f.Image
-	} else if f.Deploy.Image == "" {
-		f.Deploy.Image, err = f.ImageName()
-		if err != nil {
-			return "", f, err
-		}
-	}
-
 	// Build and deploy function using Pipeline
-	url, f.Deploy.Namespace, err = c.pipelinesProvider.Run(ctx, f)
-	if err != nil {
-		return url, f, fmt.Errorf("failed to run pipeline: %w", err)
-	}
-	return url, f, nil
+	return c.pipelinesProvider.Run(ctx, f)
 }
 
 // ConfigurePAC generates Pipeline resources on the local filesystem,
@@ -1369,8 +1351,8 @@ func (n *noopDescriber) Describe(context.Context, string, string) (Instance, err
 // PipelinesProvider
 type noopPipelinesProvider struct{}
 
-func (n *noopPipelinesProvider) Run(ctx context.Context, _ Function) (string, string, error) {
-	return "", "", nil
+func (n *noopPipelinesProvider) Run(ctx context.Context, f Function) (string, Function, error) {
+	return "", f, nil
 }
 func (n *noopPipelinesProvider) Remove(ctx context.Context, _ Function) error { return nil }
 func (n *noopPipelinesProvider) ConfigurePAC(ctx context.Context, _ Function, _ any) error {

pkg/functions/client_test.go

@@ -1437,9 +1437,10 @@ func TestClient_Pipelines_Deploy_Namespace(t *testing.T) {
 	defer rm()
 
 	pprovider := mock.NewPipelinesProvider()
-	pprovider.RunFn = func(f fn.Function) (string, string, error) {
-		// simulate function getting deployed here and return namespace
-		return "", f.Namespace, nil
+	pprovider.RunFn = func(f fn.Function) (string, fn.Function, error) {
+		// simulate function being deployed
+		f.Deploy.Namespace = f.Namespace
+		return "", f, nil
 	}
 
 	client := fn.New(

pkg/knative/deployer.go

@@ -111,7 +111,22 @@ func (d *Deployer) isImageInPrivateRegistry(ctx context.Context, client clientse
 	return false
 }
 
+func onClusterFix(f fn.Function) fn.Function {
+	// This only exists because of a bootstapping problem with On-Cluster
+	// builds:  It appears that, when sending a function to be built on-cluster
+	// the target namespace is not being transmitted in the pipeline
+	// configuration.  We should figure out how to transmit this information
+	// to the pipeline run for initial builds.  This is a new problem because
+	// earlier versions of this logic relied entirely on the current
+	// kubernetes context.
+	if f.Namespace == "" && f.Deploy.Namespace == "" {
+		f.Namespace, _ = k8s.GetDefaultNamespace()
+	}
+	return f
+}
+
 func (d *Deployer) Deploy(ctx context.Context, f fn.Function) (fn.DeploymentResult, error) {
+	f = onClusterFix(f)
 	// Choosing f.Namespace vs f.Deploy.Namespace:
 	// This is minimal logic currently required of all deployer impls.
 	// If f.Namespace is defined, this is the (possibly new) target

pkg/mock/pipelines_provider.go

@@ -9,7 +9,7 @@ import (
 
 type PipelinesProvider struct {
 	RunInvoked          bool
-	RunFn               func(fn.Function) (string, string, error)
+	RunFn               func(fn.Function) (string, fn.Function, error)
 	RemoveInvoked       bool
 	RemoveFn            func(fn.Function) error
 	ConfigurePACInvoked bool
@@ -20,25 +20,41 @@ type PipelinesProvider struct {
 
 func NewPipelinesProvider() *PipelinesProvider {
 	return &PipelinesProvider{
-		RunFn: func(f fn.Function) (x string, namespace string, err error) {
+		RunFn: func(f fn.Function) (string, fn.Function, error) {
 			// the minimum necessary logic for a deployer, which should be
-			// confirmed by tests in the respective implementations.
+			// confirmed by tests in the respective implementations, is to
+			// return the function with f.Deploy.* values set reflecting the
+			// now deployed state of the function.
+			if f.Namespace == "" && f.Deploy.Namespace == "" {
+				return "", f, errors.New("namespace required for initial deployment")
+			}
+
+			// fabricate that we deployed it to the newly requested namespace
 			if f.Namespace != "" {
-				namespace = f.Namespace
-			} else if f.Deploy.Namespace != "" {
-				namespace = f.Deploy.Namespace
+				f.Deploy.Namespace = f.Namespace
+			}
+
+			// fabricate that we deployed the requested image or generated
+			// it as needed
+			var err error
+			if f.Image != "" {
+				f.Deploy.Image = f.Image
 			} else {
-				err = errors.New("namespace required for initial deployment")
+				if f.Deploy.Image, err = f.ImageName(); err != nil {
+					return "", f, err
+				}
 			}
-			return
+
+			return "", f, nil
+
 		},
 		RemoveFn:       func(fn.Function) error { return nil },
 		ConfigurePACFn: func(fn.Function) error { return nil },
 		RemovePACFn:    func(fn.Function) error { return nil },
 	}
 }
 
-func (p *PipelinesProvider) Run(ctx context.Context, f fn.Function) (string, string, error) {
+func (p *PipelinesProvider) Run(ctx context.Context, f fn.Function) (string, fn.Function, error) {
 	p.RunInvoked = true
 	return p.RunFn(f)
 }

pkg/pipelines/tekton/client.go

@@ -13,27 +13,19 @@ const (
 	DefaultWaitingTimeout = 120 * time.Second
 )
 
-// NewTektonClientAndResolvedNamespace returns TektonV1beta1Client,namespace,error
-func NewTektonClientAndResolvedNamespace(namespace string) (*v1beta1.TektonV1beta1Client, string, error) {
-	var err error
-	if namespace == "" {
-		namespace, err = k8s.GetDefaultNamespace()
-		if err != nil {
-			return nil, "", err
-		}
-	}
-
+// NewTektonClient returns TektonV1beta1Client for namespace
+func NewTektonClient(namespace string) (*v1beta1.TektonV1beta1Client, error) {
 	restConfig, err := k8s.GetClientConfig().ClientConfig()
 	if err != nil {
-		return nil, "", fmt.Errorf("failed to create new tekton client: %w", err)
+		return nil, fmt.Errorf("failed to create new tekton client: %w", err)
 	}
 
 	client, err := v1beta1.NewForConfig(restConfig)
 	if err != nil {
-		return nil, "", fmt.Errorf("failed to create new tekton client: %v", err)
+		return nil, fmt.Errorf("failed to create new tekton client: %v", err)
 	}
 
-	return client, namespace, nil
+	return client, nil
 }
 
 func NewTektonClients() (*cli.Clients, error) {

pkg/pipelines/tekton/pipelines_integration_test.go

@@ -38,8 +38,6 @@ var testCP = func(_ context.Context, _ string) (docker.Credentials, error) {
 	return docker.Credentials{
 		Username: "",
 		Password: "",
-		// Username: "alice",
-		// Password: "alice-registry-token", Careful not to commit this.
 	}, nil
 }