Repository: redhat-developer/service-binding-operator (go)

Issue:
Service binding update fails compilation when used as lib
After updating from 0.9.0 to 0.10.0 I'm getting

```
go install -trimpath  -ldflags "-X github.com/redhat-developer/app-services-cli/internal/build.MASSSORedirectPath="mas-sso-callback" -X github.com/redhat-developer/app-services-cli/internal/build.SSORedirectPath="sso-redhat-callback" -X github.com/redhat-developer/app-services-cli/internal/build.DefaultPageNumber="1" -X github.com/redhat-developer/app-services-cli/internal/build.DefaultPageSize="10" -X github.com/redhat-developer/app-services-cli/internal/build.TermsReviewSiteCode="ocm" -X github.com/redhat-developer/app-services-cli/internal/build.TermsReviewEventCode="onlineService" -X github.com/redhat-developer/app-services-cli/internal/build.RepositoryName="app-services-cli" -X github.com/redhat-developer/app-services-cli/internal/build.RepositoryOwner="redhat-developer" -X github.com/redhat-developer/app-services-cli/internal/build.Version="dev" " ./cmd/rhoas
# github.com/redhat-developer/service-binding-operator/apis/binding/v1alpha1
../../../go/pkg/mod/github.com/redhat-developer/service-binding-operator@v0.10.0/apis/binding/v1alpha1/servicebinding_webhook.go:66:19: invalid operation: req.AdmissionRequest.Operation == v1beta1.Create (mismatched types "k8s.io/api/admission/v1".Operation and v1beta1.Operation)
../../../go/pkg/mod/github.com/redhat-developer/service-binding-operator@v0.10.0/apis/binding/v1alpha1/servicebinding_webhook.go:66:54: invalid operation: req.AdmissionRequest.Operation == v1beta1.Update (mismatched types "k8s.io/api/admission/v1".Operation and v1beta1.Operation)
`````

Using SBO as library to build binding. Webhook is not used in any of this.

### What is the SBO version used?

0.10.0



Pull request:
fix compile error when using SBO as library with newer controller-runtime
SBO uses currently controller-runtime 0.6.4 and there the admission webhook `Request` struct
uses v1beta1 API. Later controller-runtime versions use `v1` version, and this
turned to generate compile error when SBO is included as dependency in a project
using latest controller-runtime as well.

The root cause for the issue is that the admission webhook is part of `api` package,
and therefore needs to be complied even if not used in the project.

There is no reasons why the webhook code need to be a part of `api` package, and
the issue is fixed by moving it in separate `apis/webhooks` package.

Two new test projects are added so that we can catch regressions.

Fixes #1022 

Patch:
diff --git a/apis/binding/v1alpha1/servicebinding_webhook.go b/apis/binding/v1alpha1/servicebinding_webhook.go
index 08c409c8..074b1a78 100644
--- a/apis/binding/v1alpha1/servicebinding_webhook.go
+++ b/apis/binding/v1alpha1/servicebinding_webhook.go
@@ -14,77 +14,23 @@ limitations under the License.
 package v1alpha1
 
 import (
-	"context"
 	"errors"
-	"github.com/go-logr/logr"
 	"github.com/redhat-developer/service-binding-operator/apis"
-	"k8s.io/api/admission/v1beta1"
-	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
-	"net/http"
 	ctrl "sigs.k8s.io/controller-runtime"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
-	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 )
 
 // log is for logging in this package.
 var log = logf.Log.WithName("WebHook ServiceBinding")
 
-func (r *ServiceBinding) SetupWebhookWithManager(mgr ctrl.Manager, serviceAccountName string) error {
-	mgr.GetWebhookServer().Register("/mutate-servicebinding", &webhook.Admission{
-		Handler: &admisionHandler{serviceAccountName: serviceAccountName},
-	})
+func (r *ServiceBinding) SetupWebhookWithManager(mgr ctrl.Manager) error {
 	return ctrl.NewWebhookManagedBy(mgr).
 		For(r).
 		Complete()
 }
 
-// EDIT THIS FILE!  THIS IS SCAFFOLDING FOR YOU TO OWN!
-
-// +kubebuilder:webhook:path=/mutate-servicebinding,mutating=true,failurePolicy=fail,sideEffects=None,groups=binding.operators.coreos.com,resources=servicebindings,verbs=create;update,versions=v1alpha1,name=mservicebinding.kb.io,admissionReviewVersions={v1beta1}
-// +kubebuilder:webhook:path=/mutate-servicebinding,mutating=true,failurePolicy=fail,sideEffects=None,groups=servicebinding.io,resources=servicebindings,verbs=create;update,versions=v1alpha3,name=mspec-servicebinding.kb.io,admissionReviewVersions={v1beta1}
-
-type admisionHandler struct {
-	decoder            *admission.Decoder
-	log                logr.Logger
-	serviceAccountName string
-}
-
-var _ webhook.AdmissionHandler = &admisionHandler{}
-
-// Default implements webhook.Defaulter so a webhook will be registered for the type
-func (ah *admisionHandler) Handle(ctx context.Context, req admission.Request) admission.Response {
-	if req.UserInfo.Username == ah.serviceAccountName {
-		return admission.Allowed("ok")
-	}
-	sb := &unstructured.Unstructured{}
-	err := ah.decoder.Decode(req, sb)
-	if err != nil {
-		return admission.Errored(http.StatusBadRequest, err)
-	}
-	if req.Operation == v1beta1.Create || req.Operation == v1beta1.Update {
-		apis.SetRequester(sb, req.UserInfo)
-	} else {
-		return admission.Allowed("ok")
-	}
-	marshaledSB, err := sb.MarshalJSON()
-	if err != nil {
-		return admission.Errored(http.StatusInternalServerError, err)
-	}
-	return admission.PatchResponseFromRaw(req.Object.Raw, marshaledSB)
-}
-
-func (ah *admisionHandler) InjectDecoder(decoder *admission.Decoder) error {
-	ah.decoder = decoder
-	return nil
-}
-
-func (ah *admisionHandler) InjectLogger(l logr.Logger) error {
-	ah.log = l
-	return nil
-}
-
 // +kubebuilder:webhook:path=/validate-binding-operators-coreos-com-v1alpha1-servicebinding,mutating=false,failurePolicy=fail,sideEffects=None,groups=binding.operators.coreos.com,resources=servicebindings,verbs=update,versions=v1alpha1,name=vservicebinding.kb.io,admissionReviewVersions={v1beta1}
 
 var _ webhook.Validator = &ServiceBinding{}
diff --git a/apis/webhooks/admission.go b/apis/webhooks/admission.go
new file mode 100644
index 00000000..56fd267f
--- /dev/null
+++ b/apis/webhooks/admission.go
@@ -0,0 +1,61 @@
+package webhooks
+
+import (
+	"context"
+	"github.com/go-logr/logr"
+	"github.com/redhat-developer/service-binding-operator/apis"
+	"k8s.io/api/admission/v1beta1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"net/http"
+	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/webhook"
+	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
+)
+
+func SetupWithManager(mgr ctrl.Manager, serviceAccountName string) {
+	mgr.GetWebhookServer().Register("/mutate-servicebinding", &webhook.Admission{
+		Handler: &admissionHandler{serviceAccountName: serviceAccountName},
+	})
+}
+
+// +kubebuilder:webhook:path=/mutate-servicebinding,mutating=true,failurePolicy=fail,sideEffects=None,groups=binding.operators.coreos.com,resources=servicebindings,verbs=create;update,versions=v1alpha1,name=mservicebinding.kb.io,admissionReviewVersions={v1beta1}
+// +kubebuilder:webhook:path=/mutate-servicebinding,mutating=true,failurePolicy=fail,sideEffects=None,groups=servicebinding.io,resources=servicebindings,verbs=create;update,versions=v1alpha3,name=mspec-servicebinding.kb.io,admissionReviewVersions={v1beta1}
+
+type admissionHandler struct {
+	decoder            *admission.Decoder
+	log                logr.Logger
+	serviceAccountName string
+}
+
+var _ webhook.AdmissionHandler = &admissionHandler{}
+
+func (ah *admissionHandler) Handle(ctx context.Context, req admission.Request) admission.Response {
+	if req.UserInfo.Username == ah.serviceAccountName {
+		return admission.Allowed("ok")
+	}
+	sb := &unstructured.Unstructured{}
+	err := ah.decoder.Decode(req, sb)
+	if err != nil {
+		return admission.Errored(http.StatusBadRequest, err)
+	}
+	if req.Operation == v1beta1.Create || req.Operation == v1beta1.Update {
+		apis.SetRequester(sb, req.UserInfo)
+	} else {
+		return admission.Allowed("ok")
+	}
+	marshaledSB, err := sb.MarshalJSON()
+	if err != nil {
+		return admission.Errored(http.StatusInternalServerError, err)
+	}
+	return admission.PatchResponseFromRaw(req.Object.Raw, marshaledSB)
+}
+
+func (ah *admissionHandler) InjectDecoder(decoder *admission.Decoder) error {
+	ah.decoder = decoder
+	return nil
+}
+
+func (ah *admissionHandler) InjectLogger(l logr.Logger) error {
+	ah.log = l
+	return nil
+}
diff --git a/main.go b/main.go
index 65ae5370..338d0a60 100644
--- a/main.go
+++ b/main.go
@@ -19,6 +19,7 @@ package main
 import (
 	"flag"
 	"fmt"
+	"github.com/redhat-developer/service-binding-operator/apis/webhooks"
 	v1apiextensions "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	v1beta1apiextensions "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
 	"os"
@@ -140,10 +141,11 @@ func main() {
 		os.Exit(1)
 	}
 
-	if err = (&v1alpha1.ServiceBinding{}).SetupWebhookWithManager(mgr, serviceAccountName); err != nil {
+	if err = (&v1alpha1.ServiceBinding{}).SetupWebhookWithManager(mgr); err != nil {
 		setupLog.Error(err, "unable to create webhook", "webhook", "ServiceBinding")
 		os.Exit(1)
 	}
+	webhooks.SetupWithManager(mgr, serviceAccountName)
 	if err = (&specv1alpha2.ServiceBinding{}).SetupWebhookWithManager(mgr); err != nil {
 		setupLog.Error(err, "unable to create webhook", "webhook", "SPEC ServiceBinding")
 		os.Exit(1)


Test patch:
diff --git a/test/_projects/api-client/.gitignore b/test/_projects/api-client/.gitignore
new file mode 100644
index 00000000..eff50e20
--- /dev/null
+++ b/test/_projects/api-client/.gitignore
@@ -0,0 +1,2 @@
+go.sum
+main
\ No newline at end of file
diff --git a/test/_projects/api-client/go.mod b/test/_projects/api-client/go.mod
new file mode 100644
index 00000000..74e4973a
--- /dev/null
+++ b/test/_projects/api-client/go.mod
@@ -0,0 +1,10 @@
+module api-client
+
+go 1.15
+
+require (
+	github.com/redhat-developer/service-binding-operator v0.0.0
+	k8s.io/apimachinery v0.19.2
+)
+
+replace github.com/redhat-developer/service-binding-operator => ../../..
diff --git a/test/_projects/api-client/main.go b/test/_projects/api-client/main.go
new file mode 100644
index 00000000..98fc74bb
--- /dev/null
+++ b/test/_projects/api-client/main.go
@@ -0,0 +1,22 @@
+package main
+
+import (
+	bindingapi "github.com/redhat-developer/service-binding-operator/apis/binding/v1alpha1"
+
+	specv1alpha2 "github.com/redhat-developer/service-binding-operator/apis/spec/v1alpha3"
+	"k8s.io/apimachinery/pkg/runtime"
+)
+
+var (
+	scheme   = runtime.NewScheme()
+)
+
+
+func main() {
+	binding := &bindingapi.ServiceBinding{}
+	binding.Name = "sb1"
+
+	specBinding := &specv1alpha2.ServiceBinding{}
+	specBinding.Name = "sb2"
+
+}
\ No newline at end of file
diff --git a/test/_projects/api-controller/.gitignore b/test/_projects/api-controller/.gitignore
new file mode 100644
index 00000000..eff50e20
--- /dev/null
+++ b/test/_projects/api-controller/.gitignore
@@ -0,0 +1,2 @@
+go.sum
+main
\ No newline at end of file
diff --git a/test/_projects/api-controller/go.mod b/test/_projects/api-controller/go.mod
new file mode 100644
index 00000000..7ffe7c67
--- /dev/null
+++ b/test/_projects/api-controller/go.mod
@@ -0,0 +1,11 @@
+module api-controller
+
+go 1.15
+
+require (
+	github.com/redhat-developer/service-binding-operator v0.0.0
+	k8s.io/apimachinery v0.22.1
+	sigs.k8s.io/controller-runtime v0.10.0
+)
+
+replace github.com/redhat-developer/service-binding-operator => ../../..
diff --git a/test/_projects/api-controller/main.go b/test/_projects/api-controller/main.go
new file mode 100644
index 00000000..9fd369e3
--- /dev/null
+++ b/test/_projects/api-controller/main.go
@@ -0,0 +1,35 @@
+package main
+
+import (
+	"context"
+	bindingapi "github.com/redhat-developer/service-binding-operator/apis/binding/v1alpha1"
+
+	specv1alpha2 "github.com/redhat-developer/service-binding-operator/apis/spec/v1alpha3"
+	"k8s.io/apimachinery/pkg/runtime"
+	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
+
+	ctrl "sigs.k8s.io/controller-runtime"
+)
+
+var (
+	scheme   = runtime.NewScheme()
+)
+
+func init() {
+	utilruntime.Must(bindingapi.AddToScheme(scheme))
+	utilruntime.Must(specv1alpha2.AddToScheme(scheme))
+}
+
+func main() {
+	binding := &bindingapi.ServiceBinding{}
+	binding.Name = "sb1"
+
+	specBinding := &specv1alpha2.ServiceBinding{}
+	specBinding.Name = "sb2"
+
+	m, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{})
+	if err != nil {
+		panic(err)
+	}
+	m.Start(context.Background())
+}
\ No newline at end of file
diff --git a/test/library_test.go b/test/library_test.go
new file mode 100644
index 00000000..574360fb
--- /dev/null
+++ b/test/library_test.go
@@ -0,0 +1,33 @@
+package test
+
+import (
+	. "github.com/onsi/ginkgo/extensions/table"
+	"os/exec"
+	"path/filepath"
+	"testing"
+
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+)
+
+func TestAPIUsageAsLibrary(t *testing.T) {
+	RegisterFailHandler(Fail)
+	RunSpecs(t, "Library Suite")
+}
+
+var _ = DescribeTable("Using service binding API as library", func(projectPath string) {
+
+	cmd := exec.Command("sh", "-c", "go mod download && go mod tidy && go build -o main")
+	absProjectPath, err := filepath.Abs(projectPath)
+	Expect(err).NotTo(HaveOccurred())
+
+	cmd.Dir = absProjectPath
+	cmd.Stdout = GinkgoWriter
+	cmd.Stderr = GinkgoWriter
+	err = cmd.Run()
+
+	Expect(err).NotTo(HaveOccurred())
+},
+	Entry("from controller", "_projects/api-controller"),
+	Entry("from application consuming just API", "_projects/api-client"),
+)