Add webhook server to tests (#2873)

* Add webhook server to tests
* fix config for webhoook service
* Fix logger in webhook manager
* Use interface for webhook manager
* single reference for router url
* Cleanup token code

---------

Signed-off-by: Sanket Sudake <sanketsudake@gmail.com>

.codecov.yml

@@ -1,6 +1,6 @@
 ignore:
   - "charts"
-  - "test/tests"
+  - "test/"
   - "tools"
   - "pkg/apis/genclient"
 coverage:

.github/workflows/lint.yaml

@@ -24,7 +24,7 @@ env:
 jobs:
   lint:
     runs-on: ubuntu-latest
-    if: ${{ !contains(github.event.pull_request.labels.*.name, 'skip-ci') }}
+    # if: ${{ !contains(github.event.pull_request.labels.*.name, 'skip-ci') }}
     steps:
       - name: Check out code
         uses: actions/checkout@v4

cmd/fission-bundle/main.go

@@ -26,6 +26,7 @@ import (
 	docopt "github.com/docopt/docopt-go"
 	"go.uber.org/zap"
 	"sigs.k8s.io/controller-runtime/pkg/manager/signals"
+	cnwebhook "sigs.k8s.io/controller-runtime/pkg/webhook"
 
 	"github.com/fission/fission/cmd/fission-bundle/mqtrigger"
 	"github.com/fission/fission/pkg/buildermgr"
@@ -48,8 +49,10 @@ import (
 )
 
 // runWebhook starts admission webhook server
-func runWebhook(ctx context.Context, logger *zap.Logger, port int) error {
-	return webhook.Start(ctx, logger, port)
+func runWebhook(ctx context.Context, clientGen crd.ClientGeneratorInterface, logger *zap.Logger, port int) error {
+	return webhook.Start(ctx, clientGen, logger, cnwebhook.Options{
+		Port: port,
+	})
 }
 
 func runCanaryConfigServer(ctx context.Context, clientGen crd.ClientGeneratorInterface, logger *zap.Logger, mgr manager.Interface) error {
@@ -235,7 +238,7 @@ Options:
 
 	if arguments["--webhookPort"] != nil {
 		port := getPort(logger, arguments["--webhookPort"])
-		err = runWebhook(ctx, logger, port)
+		err = runWebhook(ctx, clientGen, logger, port)
 		logger.Error("webhook server exited:", zap.Error(err))
 		return
 	}

cmd/fission-cli/app/app.go

@@ -81,7 +81,7 @@ func App(clientOptions cmd.ClientOptions) *cobra.Command {
 	})
 
 	wrapper.SetFlags(rootCmd, flag.FlagSet{
-		Global: []flag.Flag{flag.GlobalServer, flag.GlobalVerbosity, flag.KubeContext, flag.Namespace},
+		Global: []flag.Flag{flag.GlobalVerbosity, flag.KubeContext, flag.Namespace},
 	})
 
 	groups := helptemplate.CommandGroups{}

go.mod

@@ -13,6 +13,7 @@ require (
 	github.com/fatih/color v1.15.0
 	github.com/fsnotify/fsnotify v1.7.0
 	github.com/go-git/go-git/v5 v5.10.0
+	github.com/go-logr/zapr v1.2.4
 	github.com/golang-jwt/jwt/v4 v4.5.0
 	github.com/google/go-cmp v0.6.0
 	github.com/google/uuid v1.4.0
@@ -90,7 +91,6 @@ require (
 	github.com/go-ini/ini v1.66.4 // indirect
 	github.com/go-logr/logr v1.2.4 // indirect
 	github.com/go-logr/stdr v1.2.2 // indirect
-	github.com/go-logr/zapr v1.2.4 // indirect
 	github.com/go-openapi/jsonpointer v0.19.6 // indirect
 	github.com/go-openapi/jsonreference v0.20.2 // indirect
 	github.com/go-openapi/swag v0.22.3 // indirect

pkg/crd/client.go

@@ -37,6 +37,7 @@ import (
 
 type (
 	ClientGeneratorInterface interface {
+		GetRestConfig() (*rest.Config, error)
 		GetFissionClient() (versioned.Interface, error)
 		GetKubernetesClient() (kubernetes.Interface, error)
 		GetApiExtensionsClient() (apiextensionsclient.Interface, error)
@@ -62,6 +63,10 @@ func (cg *ClientGenerator) getRestConfig() (*rest.Config, error) {
 	return cg.restConfig, nil
 }
 
+func (cg *ClientGenerator) GetRestConfig() (*rest.Config, error) {
+	return cg.getRestConfig()
+}
+
 func (cg *ClientGenerator) GetFissionClient() (versioned.Interface, error) {
 	config, err := cg.getRestConfig()
 	if err != nil {
@@ -121,11 +126,10 @@ func WaitForFunctionCRDs(ctx context.Context, logger *zap.Logger, fissionClient
 	for {
 		fi := fissionClient.CoreV1().Functions(defaultNs)
 		_, err := fi.List(ctx, metav1.ListOptions{})
-		if err != nil {
-			time.Sleep(100 * time.Millisecond)
-		} else {
+		if err == nil {
 			return nil
 		}
+		time.Sleep(100 * time.Millisecond)
 
 		if time.Since(start) > 30*time.Second {
 			return fmt.Errorf("timeout waiting for function CRD access")

pkg/fission-cli/cmd/function/test.go

@@ -66,31 +66,21 @@ func (opts *TestSubCommand) do(input cli.Input) error {
 		Namespace: namespace,
 	}
 
-	routerURL := os.Getenv("FISSION_ROUTER_URL")
-	if len(routerURL) == 0 {
-		// Portforward to the fission router
-		localRouterPort, err := util.SetupPortForward(input.Context(), opts.Client(), util.GetFissionNamespace(), "application=fission-router")
-		if err != nil {
-			return err
-		}
-		routerURL = "http://127.0.0.1:" + localRouterPort
+	routerURL, err := util.GetRouterURL(input.Context(), opts.Client())
+	if err != nil {
+		return errors.Wrap(err, "error getting router URL")
 	}
-	fnURL := routerURL + util.UrlForFunction(m.Name, m.Namespace)
+	fnURI := util.UrlForFunction(m.Name, m.Namespace)
 	if input.IsSet(flagkey.FnSubPath) {
 		subPath := input.String(flagkey.FnSubPath)
 		if !strings.HasPrefix(subPath, "/") {
-			fnURL = fnURL + "/" + subPath
+			fnURI = fnURI + "/" + subPath
 		} else {
-			fnURL = fnURL + subPath
+			fnURI = fnURI + subPath
 		}
 	}
-
-	functionUrl, err := url.Parse(fnURL)
-	if err != nil {
-		return err
-	}
-
-	console.Verbose(2, "Function test url: %v", functionUrl.String())
+	fnURL := routerURL.JoinPath(fnURI)
+	console.Verbose(2, "Function test url: %v", fnURL.String())
 
 	queryParams := input.StringSlice(flagkey.FnTestQuery)
 	if len(queryParams) > 0 {
@@ -109,7 +99,7 @@ func (opts *TestSubCommand) do(input cli.Input) error {
 			}
 			query.Set(key, value)
 		}
-		functionUrl.RawQuery = query.Encode()
+		fnURL.RawQuery = query.Encode()
 	}
 
 	var (
@@ -145,7 +135,7 @@ func (opts *TestSubCommand) do(input cli.Input) error {
 	if err != nil {
 		return err
 	}
-	resp, err := doHTTPRequest(ctx, functionUrl.String(),
+	resp, err := doHTTPRequest(ctx, fnURL.String(),
 		input.StringSlice(flagkey.FnTestHeader),
 		method,
 		input.String(flagkey.FnTestBody))

pkg/fission-cli/cmd/token/create.go

@@ -22,14 +22,14 @@ import (
 	"fmt"
 	"io"
 	"net/http"
-	"net/url"
 	"os"
 
 	"github.com/pkg/errors"
 
 	fv1 "github.com/fission/fission/pkg/apis/core/v1"
 	"github.com/fission/fission/pkg/fission-cli/cliwrapper/cli"
 	"github.com/fission/fission/pkg/fission-cli/cmd"
+	"github.com/fission/fission/pkg/fission-cli/console"
 	flagkey "github.com/fission/fission/pkg/fission-cli/flag/key"
 	"github.com/fission/fission/pkg/fission-cli/util"
 )
@@ -64,26 +64,19 @@ func (opts *CreateSubCommand) run(input cli.Input) error {
 
 	jsonValue, _ := json.Marshal(values)
 
-	// Portforward to the fission router
-	localRouterPort, err := util.SetupPortForward(input.Context(), opts.Client(), util.GetFissionNamespace(), "application=fission-router")
-	if err != nil {
-		return err
-	}
-
 	authURI, _ := os.LookupEnv("FISSION_AUTH_URI")
-
 	if input.IsSet(flagkey.TokAuthURI) {
 		authURI = input.String(flagkey.TokAuthURI)
 	}
-
 	if len(authURI) == 0 {
 		authURI = util.FISSION_AUTH_URI
 	}
-
-	relativeURL, _ := url.Parse(authURI)
-	serverURL, _ := url.Parse("http://127.0.0.1:" + localRouterPort)
-	authAuthenticatorUrl := serverURL.ResolveReference(relativeURL)
-
+	routerURL, err := util.GetRouterURL(input.Context(), opts.Client())
+	if err != nil {
+		return errors.Wrap(err, "error getting router URL")
+	}
+	authAuthenticatorUrl := routerURL.JoinPath(authURI)
+	console.Verbose(2, "Auth URI: %s", authAuthenticatorUrl.String())
 	resp, err := http.Post(authAuthenticatorUrl.String(), "application/json", bytes.NewBuffer(jsonValue))
 	if err != nil {
 		return err

pkg/fission-cli/flag/flag.go

@@ -68,7 +68,6 @@ const (
 
 var (
 	GlobalVerbosity = Flag{Type: Int, Name: flagkey.Verbosity, Short: "v", Usage: "CLI verbosity (0 is quiet, 1 is the default, 2 is verbose)", DefaultValue: 1}
-	GlobalServer    = Flag{Type: String, Name: flagkey.Server, Usage: "Server URL"}
 
 	ClientOnly = Flag{Type: Bool, Name: flagkey.ClientOnly, Usage: "If set, the CLI won't connect to remote server"}
 

pkg/fission-cli/util/util.go

@@ -170,7 +170,7 @@ func GetVersion(ctx context.Context, input cli.Input, cmdClient cmd.Client) info
 func GetServerInfo(input cli.Input, cmdClient cmd.Client) *info.ServerInfo {
 
 	var serverInfo info.ServerInfo
-	serverURL, err := getRouterURL(input.Context(), cmdClient)
+	serverURL, err := GetRouterURL(input.Context(), cmdClient)
 	if err != nil {
 		console.Warn("could not connect to server")
 		return &serverInfo
@@ -212,7 +212,12 @@ func GetServerInfo(input cli.Input, cmdClient cmd.Client) *info.ServerInfo {
 	return &serverInfo
 }
 
-func getRouterURL(ctx context.Context, cmdClient cmd.Client) (serverURL *url.URL, err error) {
+func GetRouterURL(ctx context.Context, cmdClient cmd.Client) (serverURL *url.URL, err error) {
+	routerURL := os.Getenv("FISSION_ROUTER_URL")
+	if len(routerURL) > 0 {
+		return url.Parse(routerURL)
+	}
+
 	// Portforward to the fission router
 	localRouterPort, err := SetupPortForward(ctx, cmdClient, GetFissionNamespace(), "application=fission-router")
 	if err != nil {
@@ -226,26 +231,6 @@ func getRouterURL(ctx context.Context, cmdClient cmd.Client) (serverURL *url.URL
 	return serverURL, err
 }
 
-func GetServerURL(input cli.Input, client cmd.Client) (serverUrl string, err error) {
-	serverUrl = input.GlobalString(flagkey.Server)
-	if len(serverUrl) == 0 {
-		// starts local portforwarder etc.
-		serverUrl, err = GetApplicationUrl(input.Context(), client, "application=fission-api")
-		if err != nil {
-			return "", err
-		}
-	}
-
-	isHTTPS := strings.Index(serverUrl, "https://") == 0
-	isHTTP := strings.Index(serverUrl, "http://") == 0
-
-	if !(isHTTP || isHTTPS) {
-		serverUrl = "http://" + serverUrl
-	}
-
-	return serverUrl, nil
-}
-
 func GetResourceReqs(input cli.Input, resReqs *v1.ResourceRequirements) (*v1.ResourceRequirements, error) {
 	r := &v1.ResourceRequirements{}
 

pkg/webhook/admission-webhook.go

@@ -18,19 +18,23 @@ package webhook
 
 import (
 	"context"
+	"fmt"
 	"os"
 
 	// Import all Kubernetes client auth plugins (e.g. Azure, GCP, OIDC, etc.)
 	// to ensure that exec-entrypoint and run can make use of them.
 
+	"github.com/go-logr/zapr"
 	"go.uber.org/zap"
 	_ "k8s.io/client-go/plugin/pkg/client/auth"
-	"sigs.k8s.io/controller-runtime/pkg/client/config"
+	"sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/manager"
+
 	metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
 
 	v1 "github.com/fission/fission/pkg/apis/core/v1"
+	"github.com/fission/fission/pkg/crd"
 	"github.com/fission/fission/pkg/generated/clientset/versioned/scheme"
 	//+kubebuilder:scaffold:imports
 )
@@ -39,26 +43,33 @@ type WebhookInjector interface {
 	SetupWebhookWithManager(mgr manager.Manager) error
 }
 
-func Start(ctx context.Context, logger *zap.Logger, port int) (err error) {
-
+func Start(ctx context.Context, clientGen crd.ClientGeneratorInterface, logger *zap.Logger, options webhook.Options) (err error) {
 	wLogger := logger.Named("webhook")
+	zaprLogger := zapr.NewLogger(logger)
+	log.SetLogger(zaprLogger)
 
 	metricsAddr := os.Getenv("METRICS_ADDR")
 	if metricsAddr == "" {
 		metricsAddr = ":8080"
 	}
-
+	if metricsAddr[0] != ':' {
+		metricsAddr = fmt.Sprintf(":%s", metricsAddr)
+	}
 	mgrOpt := manager.Options{
 		Scheme: scheme.Scheme,
 		Metrics: metricsserver.Options{
 			BindAddress: metricsAddr,
 		},
-		WebhookServer: webhook.NewServer(webhook.Options{
-			Port: port,
-		}),
+		WebhookServer: webhook.NewServer(options),
+		Logger:        zaprLogger,
+	}
+	restConfig, err := clientGen.GetRestConfig()
+	if err != nil {
+		wLogger.Error("unable to get rest config", zap.Error(err))
+		return err
 	}
 	// Setup a Manager
-	mgr, err := manager.New(config.GetConfigOrDie(), mgrOpt)
+	mgr, err := manager.New(restConfig, mgrOpt)
 	if err != nil {
 		wLogger.Error("unable to set up overall controller manager", zap.Error(err))
 		return err

test/e2e/cli/cli_test.go

@@ -3,9 +3,11 @@ package cli_test
 import (
 	"context"
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/require"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/wait"
 
 	v1 "github.com/fission/fission/pkg/apis/core/v1"
 	"github.com/fission/fission/pkg/utils/manager"
@@ -20,6 +22,7 @@ func TestFissionCLI(t *testing.T) {
 	defer mgr.Wait()
 
 	f := framework.NewFramework()
+	defer f.Logger().Sync()
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
 	err := f.Start(ctx)
@@ -28,6 +31,14 @@ func TestFissionCLI(t *testing.T) {
 	err = services.StartServices(ctx, f, mgr)
 	require.NoError(t, err)
 
+	err = wait.PollUntilContextTimeout(ctx, time.Second*5, time.Second*50, true, func(_ context.Context) (bool, error) {
+		if err := f.CheckService("webhook"); err != nil {
+			return false, nil
+		}
+		return true, nil
+	})
+	require.NoError(t, err)
+
 	fissionClient, err := f.ClientGen().GetFissionClient()
 	require.NoError(t, err)