fix: sharedmain.AppBuild webservices client not using scheme (knative#35

)

errors/errorhandling.go

@@ -27,6 +27,9 @@ import (
 	"k8s.io/apimachinery/pkg/runtime/schema"
 )
 
+// RESTClientGroupResource fake GroupResource to use errors api
+var RESTClientGroupResource = schema.GroupResource{Group: "katanomi.dev", Resource: "RESTfulClient"}
+
 //AsAPIError returns an error as a apimachinary api error
 func AsAPIError(err error) error {
 	reason := errors.ReasonForError(err)
@@ -45,11 +48,17 @@ func AsStatusCode(err error) int {
 }
 
 // AsStatusError transform resty response to status error
-func AsStatusError(response *resty.Response) error {
+func AsStatusError(response *resty.Response, grs ...schema.GroupResource) error {
+	// adding GroupResource as a "optional" parameter only
+	// should never provide more than one
+	gr := RESTClientGroupResource
+	if len(grs) > 0 {
+		gr = grs[0]
+	}
 	statusError := errors.NewGenericServerResponse(
 		response.StatusCode(),
 		response.Request.Method,
-		schema.GroupResource{},
+		gr,
 		response.Request.URL,
 		response.String(),
 		0,

examples/sample-api/main.go

@@ -35,7 +35,6 @@ import (
 )
 
 func main() {
-
 	sharedmain.App("sample-api").
 		Scheme(scheme.Scheme).
 		Log().

examples/sample-controller/.ko.yaml

@@ -1,2 +1,2 @@
 # Use :nonroot base image for all containers
-defaultBaseImage: gcr.io/distroless/static:debug
+defaultBaseImage: katanomi/distroless-static:nonroot

go.mod

@@ -9,28 +9,33 @@ require (
 	github.com/cloudevents/sdk-go/v2 v2.5.0
 	github.com/emicklei/go-restful-openapi/v2 v2.3.0
 	github.com/emicklei/go-restful/v3 v3.5.1
-	github.com/go-logr/logr v0.3.0
+	github.com/go-logr/logr v0.4.0
 	github.com/go-logr/zapr v0.2.0
+	github.com/go-openapi/spec v0.20.2 // indirect
 	github.com/go-resty/resty/v2 v2.6.0
 	github.com/golang/mock v1.6.0
-	github.com/google/go-cmp v0.5.5
+	github.com/google/go-cmp v0.5.6
+	github.com/googleapis/gnostic v0.5.3 // indirect
 	github.com/jarcoal/httpmock v1.0.8
 	github.com/onsi/ginkgo v1.14.1
-	github.com/onsi/gomega v1.10.2
+	github.com/onsi/gomega v1.10.3
 	github.com/opentracing/opentracing-go v1.1.0
-	github.com/prometheus/client_golang v1.10.0
+	github.com/prometheus/client_golang v1.11.0
+	github.com/sirupsen/logrus v1.7.0 // indirect
 	github.com/uber/jaeger-client-go v2.29.1+incompatible
 	github.com/uber/jaeger-lib v2.4.1+incompatible
-	go.uber.org/zap v1.17.0
+	go.uber.org/zap v1.18.1
 	golang.org/x/sync v0.0.0-20210220032951-036812b2e83c
 	k8s.io/api v0.20.7
 	k8s.io/apimachinery v0.20.7
-	k8s.io/client-go v0.20.2
-	knative.dev/pkg v0.0.0-20210510175900-4564797bf3b7
+	k8s.io/client-go v0.20.7
+	k8s.io/klog/v2 v2.5.0 // indirect
+	k8s.io/kube-openapi v0.0.0-20210113233702-8566a335510f // indirect
+	knative.dev/pkg v0.0.0-20210730172132-bb4aaf09c430
 	sigs.k8s.io/controller-runtime v0.8.3
 	sigs.k8s.io/yaml v1.2.0
 	yunion.io/x/log v0.0.0-20201210064738-43181789dc74 // indirect
 	yunion.io/x/pkg v0.0.0-20210218105412-13a69f60034c
 )
 
-replace go.uber.org/zap v1.17.0 => github.com/katanomi/zap v1.18.2 // indirect
+replace go.uber.org/zap => github.com/katanomi/zap v1.18.2 // indirect

restclient/statuscode.go

@@ -17,38 +17,15 @@ limitations under the License.
 package restclient
 
 import (
-	"fmt"
-	"net/http"
-
 	"github.com/go-resty/resty/v2"
-	"k8s.io/apimachinery/pkg/api/errors"
-	"k8s.io/apimachinery/pkg/runtime/schema"
+	kerrors "github.com/katanomi/pkg/errors"
 )
 
-// RESTClientGroupResource fake GroupResource to use errors api
-var RESTClientGroupResource = schema.GroupResource{Group: "katanomi.dev", Resource: "RESTfulClient"}
-
 // GetErrorFromResponse returns an error based on the response. Will do the best effort to convert
 // error responses into apimachinery errors
 func GetErrorFromResponse(resp *resty.Response, err error) error {
 	if resp.IsError() {
-		if err == nil {
-			err = fmt.Errorf(resp.String())
-		}
-		switch resp.StatusCode() {
-		case http.StatusBadRequest:
-			return errors.NewBadRequest(err.Error())
-		case http.StatusUnauthorized:
-			return errors.NewUnauthorized(err.Error())
-		case http.StatusMethodNotAllowed:
-			return errors.NewMethodNotSupported(RESTClientGroupResource, "")
-		case http.StatusInternalServerError:
-			return errors.NewInternalError(err)
-		case http.StatusRequestTimeout:
-			return errors.NewTimeoutError(err.Error(), 0)
-		default:
-			return err
-		}
+		return kerrors.AsStatusError(resp)
 	}
 	return nil
 }

sharedmain/app.go

@@ -49,6 +49,7 @@ import (
 	"knative.dev/pkg/profiling"
 	"knative.dev/pkg/system"
 	ctrl "sigs.k8s.io/controller-runtime"
+	ctrlclient "sigs.k8s.io/controller-runtime/pkg/client"
 	ctrlcluster "sigs.k8s.io/controller-runtime/pkg/cluster"
 	"sigs.k8s.io/controller-runtime/pkg/healthz"
 	ctrllog "sigs.k8s.io/controller-runtime/pkg/log"
@@ -94,6 +95,8 @@ type AppBuilder struct {
 	filters   []restful.FilterFunction
 
 	startFunc []func(context.Context) error
+
+	initClientOnce sync.Once
 }
 
 // App main constructor entrypoint for AppBuilder
@@ -124,6 +127,22 @@ func (a *AppBuilder) Scheme(scheme *runtime.Scheme) *AppBuilder {
 	return a
 }
 
+func (a *AppBuilder) initClient(clientVar ctrlclient.Client) {
+	a.initClientOnce.Do(func() {
+		if clientVar == nil {
+			cluster, err := ctrlcluster.New(a.Config, WithScheme(a.scheme))
+			if err != nil {
+				a.Logger.Fatalw("cluster client setup error", "err", err)
+			}
+			clientVar = cluster.GetClient()
+			a.startFunc = append(a.startFunc, func(ctx context.Context) error {
+				return cluster.Start(ctx)
+			})
+		}
+		a.Context = kclient.WithClient(a.Context, clientVar)
+	})
+}
+
 // Log adds logging and logger to the app
 func (a *AppBuilder) Log() *AppBuilder {
 	a.init()
@@ -142,6 +161,13 @@ func (a *AppBuilder) Log() *AppBuilder {
 	a.Logger, _ = SetupLoggerOrDie(a.Context, a.Name)
 	a.Context = logging.WithLogger(a.Context, a.Logger)
 
+	// this logger will not respect the automatic level update feature
+	// and should not be used
+	// its main purpose is to provide a logger to controller-runtime
+	zaplogger := zapr.NewLogger(a.Logger.Desugar())
+	ctrl.SetLogger(zaplogger)
+	a.Context = ctrllog.IntoContext(a.Context, zaplogger)
+
 	// watches logging config changes and reset the level manager
 	WatchLoggingConfigOrDie(a.Context, a.ConfigMapWatcher, a.Logger, a.LevelManager)
 
@@ -187,13 +213,6 @@ func (a *AppBuilder) Controllers(ctors ...Controller) *AppBuilder {
 	}
 	options.Scheme = a.scheme
 
-	// this logger will not respect the automatic level update feature
-	// and should not be used
-	// its main purpose is to provide a logger to controller-runtime
-	zaplogger := zapr.NewLogger(a.Logger.Desugar())
-	ctrl.SetLogger(zaplogger)
-	a.Context = ctrllog.IntoContext(a.Context, zaplogger)
-
 	a.Manager, err = ctrl.NewManager(a.Config, options)
 	if err != nil {
 		a.Logger.Fatalw("unable to start manager", "err", err)
@@ -206,7 +225,7 @@ func (a *AppBuilder) Controllers(ctors ...Controller) *AppBuilder {
 	}
 
 	a.Context = kmanager.WithManager(a.Context, a.Manager)
-	a.Context = kclient.WithClient(a.Context, a.Manager.GetClient())
+	a.initClient(a.Manager.GetClient())
 
 	for _, controller := range ctors {
 		name := controller.Name()
@@ -277,34 +296,15 @@ func (a *AppBuilder) Filters(filters ...restful.FilterFunction) *AppBuilder {
 	return a
 }
 
-//func (a *AppBuilder) ClientManager(mgr *kclient.Manager) *AppBuilder {
-//	a.Context = kclient.WithManager(a.Context, mgr)
-//	return a
-//}
-
 // Container adds a containers
 func (a *AppBuilder) Container(container *restful.Container) *AppBuilder {
 	a.container = container
 	return a
 }
 
 func (a *AppBuilder) Webservices(webServices ...WebService) *AppBuilder {
-
-	cluster, err := ctrlcluster.New(a.Config)
-	if err != nil {
-		a.Logger.Fatalw("cluster setup error", "err", err)
-	}
-	a.Context = kclient.WithClient(a.Context, cluster.GetClient())
-	a.startFunc = append(a.startFunc, func(ctx context.Context) error {
-		go func() {
-			err := cluster.Start(ctx)
-			if err != nil {
-				a.Logger.Fatalw("cluster start error", "err", err)
-			}
-		}()
-
-		return nil
-	})
+	// will init a client if not already initiated
+	a.initClient(nil)
 
 	for _, item := range webServices {
 		name := item.Name()

sharedmain/options.go

@@ -0,0 +1,29 @@
+/*
+Copyright 2021 The Katanomi Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package sharedmain
+
+import (
+	"k8s.io/apimachinery/pkg/runtime"
+	ctrlcluster "sigs.k8s.io/controller-runtime/pkg/cluster"
+)
+
+// WithScheme adds a scheme to cluster.Options
+func WithScheme(scheme *runtime.Scheme) ctrlcluster.Option {
+	return func(opts *ctrlcluster.Options) {
+		opts.Scheme = scheme
+	}
+}