From c949b01aac6e780bba58f7da64ff28d13c853746 Mon Sep 17 00:00:00 2001
From: JoshVanL <vleeuwenjoshua@gmail.com>
Date: Mon, 8 Jul 2019 11:56:46 +0100
Subject: [PATCH 1/2] Adds Group to CertificateRequest IssuerRef to distinguish
 controller ownership

Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>
---
 deploy/manifests/00-crds.yaml                 | 15 ++++-
 .../output/reference/api-docs/index.html      |  6 +-
 pkg/apis/certmanager/v1alpha1/types.go        |  4 +-
 .../v1alpha1/types_certificaterequest.go      |  5 +-
 .../certificaterequests/BUILD.bazel           |  1 +
 pkg/controller/certificaterequests/sync.go    |  6 ++
 .../certificaterequests/sync_test.go          | 62 ++++++++++++++++++-
 7 files changed, 92 insertions(+), 7 deletions(-)

diff --git a/deploy/manifests/00-crds.yaml b/deploy/manifests/00-crds.yaml
index 4da49d4dc..89a5154dd 100644
--- a/deploy/manifests/00-crds.yaml
+++ b/deploy/manifests/00-crds.yaml
@@ -112,6 +112,8 @@ spec:
                 with the provided name will be used. The 'name' field in this stanza
                 is required at all times.
               properties:
+                group:
+                  type: string
                 kind:
                   type: string
                 name:
@@ -279,9 +281,12 @@ spec:
                 the 'kind' field is not set, or set to 'Issuer', an Issuer resource
                 with the given name in the same namespace as the CertificateRequest
                 will be used.  If the 'kind' field is set to 'ClusterIssuer', a ClusterIssuer
-                with the provided name will be used.  The 'name' field in this stanza
-                is required at all times.
+                with the provided name will be used. The 'name' field in this stanza
+                is required at all times. The group field refers to the API group
+                of the issuer which defaults to 'certmanager.k8s.io' if empty.
               properties:
+                group:
+                  type: string
                 kind:
                   type: string
                 name:
@@ -414,6 +419,8 @@ spec:
                 Issuer, an error will be returned and the Challenge will be marked
                 as failed.
               properties:
+                group:
+                  type: string
                 kind:
                   type: string
                 name:
@@ -1243,6 +1250,8 @@ spec:
                 Issuer, an error will be returned and the Order will be marked as
                 failed.
               properties:
+                group:
+                  type: string
                 kind:
                   type: string
                 name:
@@ -1290,6 +1299,8 @@ spec:
                       is not an 'ACME' Issuer, an error will be returned and the Challenge
                       will be marked as failed.
                     properties:
+                      group:
+                        type: string
                       kind:
                         type: string
                       name:
diff --git a/docs/generated/reference/output/reference/api-docs/index.html b/docs/generated/reference/output/reference/api-docs/index.html
index fbc42f9c7..3c4a5fec7 100755
--- a/docs/generated/reference/output/reference/api-docs/index.html
+++ b/docs/generated/reference/output/reference/api-docs/index.html
@@ -2172,7 +2172,7 @@ Appears In:
 <td><code>ObjectReference</code></td>
 </tr>
 </tbody></table>
-<p>ObjectReference is a reference to an object with a given name and kind.</p>
+<p>ObjectReference is a reference to an object with a given name, kind and group.</p>
 <aside class="notice">
 Appears In:
 
@@ -2190,6 +2190,10 @@ Appears In:
 </tr>
 </thead>
 <tbody><tr>
+<td><code>group</code><br /> <em>string</em></td>
+<td></td>
+</tr>
+<tr>
 <td><code>kind</code><br /> <em>string</em></td>
 <td></td>
 </tr>
diff --git a/pkg/apis/certmanager/v1alpha1/types.go b/pkg/apis/certmanager/v1alpha1/types.go
index af2db7305..ebcf11414 100644
--- a/pkg/apis/certmanager/v1alpha1/types.go
+++ b/pkg/apis/certmanager/v1alpha1/types.go
@@ -51,11 +51,13 @@ type LocalObjectReference struct {
 	Name string `json:"name"`
 }
 
-// ObjectReference is a reference to an object with a given name and kind.
+// ObjectReference is a reference to an object with a given name, kind and group.
 type ObjectReference struct {
 	Name string `json:"name"`
 	// +optional
 	Kind string `json:"kind,omitempty"`
+	// +optional
+	Group string `json:"group,omitempty"`
 }
 
 const (
diff --git a/pkg/apis/certmanager/v1alpha1/types_certificaterequest.go b/pkg/apis/certmanager/v1alpha1/types_certificaterequest.go
index 69bb248fe..5bf5fa5dd 100644
--- a/pkg/apis/certmanager/v1alpha1/types_certificaterequest.go
+++ b/pkg/apis/certmanager/v1alpha1/types_certificaterequest.go
@@ -58,8 +58,9 @@ type CertificateRequestSpec struct {
 	// the 'kind' field is not set, or set to 'Issuer', an Issuer resource with
 	// the given name in the same namespace as the CertificateRequest will be
 	// used.  If the 'kind' field is set to 'ClusterIssuer', a ClusterIssuer with
-	// the provided name will be used.  The 'name' field in this stanza is
-	// required at all times.
+	// the provided name will be used. The 'name' field in this stanza is
+	// required at all times. The group field refers to the API group of the
+	// issuer which defaults to 'certmanager.k8s.io' if empty.
 	IssuerRef ObjectReference `json:"issuerRef"`
 
 	// Byte slice containing the PEM encoded CertificateSigningRequest
diff --git a/pkg/controller/certificaterequests/BUILD.bazel b/pkg/controller/certificaterequests/BUILD.bazel
index be48f5982..13101a7f2 100644
--- a/pkg/controller/certificaterequests/BUILD.bazel
+++ b/pkg/controller/certificaterequests/BUILD.bazel
@@ -11,6 +11,7 @@ go_library(
     visibility = ["//visibility:public"],
     deps = [
         "//pkg/api/util:go_default_library",
+        "//pkg/apis/certmanager:go_default_library",
         "//pkg/apis/certmanager/v1alpha1:go_default_library",
         "//pkg/apis/certmanager/validation:go_default_library",
         "//pkg/client/clientset/versioned:go_default_library",
diff --git a/pkg/controller/certificaterequests/sync.go b/pkg/controller/certificaterequests/sync.go
index 8e3ca6747..665857812 100644
--- a/pkg/controller/certificaterequests/sync.go
+++ b/pkg/controller/certificaterequests/sync.go
@@ -27,6 +27,7 @@ import (
 	utilerrors "k8s.io/apimachinery/pkg/util/errors"
 
 	apiutil "github.com/jetstack/cert-manager/pkg/api/util"
+	"github.com/jetstack/cert-manager/pkg/apis/certmanager"
 	"github.com/jetstack/cert-manager/pkg/apis/certmanager/v1alpha1"
 	"github.com/jetstack/cert-manager/pkg/apis/certmanager/validation"
 	"github.com/jetstack/cert-manager/pkg/issuer"
@@ -50,6 +51,11 @@ func (c *Controller) Sync(ctx context.Context, cr *v1alpha1.CertificateRequest)
 	log := logf.FromContext(ctx)
 	dbg := log.V(logf.DebugLevel)
 
+	if !(cr.Spec.IssuerRef.Group == "" || cr.Spec.IssuerRef.Group == certmanager.GroupName) {
+		dbg.Info("certificate request issuerRef group does not match certmanager group so skipping processing")
+		return nil
+	}
+
 	if apiutil.CertificateRequestHasCondition(cr, v1alpha1.CertificateRequestCondition{
 		Type:   v1alpha1.CertificateRequestConditionReady,
 		Status: v1alpha1.ConditionFalse,
diff --git a/pkg/controller/certificaterequests/sync_test.go b/pkg/controller/certificaterequests/sync_test.go
index 8f5d3298c..42fc6004c 100644
--- a/pkg/controller/certificaterequests/sync_test.go
+++ b/pkg/controller/certificaterequests/sync_test.go
@@ -198,6 +198,14 @@ func TestSync(t *testing.T) {
 	exampleEmptyCSRCR := exampleCR.DeepCopy()
 	exampleEmptyCSRCR.Spec.CSRPEM = make([]byte, 0)
 
+	exampleCRWrongIssuerRefGroup := exampleCR.DeepCopy()
+	exampleCRWrongIssuerRefGroup.Spec.IssuerRef.Group = "notcertmanager.k8s.io"
+
+	exampleCRCorrentIssuerRefGroup := exampleCRWrongIssuerRefGroup.DeepCopy()
+	exampleCRCorrentIssuerRefGroup.Spec.IssuerRef.Group = "certmanager.k8s.io"
+	exampleCRReadyConditionWithGroupRef := exampleCRReadyCondition.DeepCopy()
+	exampleCRReadyConditionWithGroupRef.Spec.IssuerRef.Group = "certmanager.k8s.io"
+
 	tests := map[string]controllerFixture{
 		"should update certificate request with CertPending if issuer does not return a response": {
 			Issuer: gen.Issuer("test",
@@ -230,7 +238,7 @@ func TestSync(t *testing.T) {
 			},
 			Err: false,
 		},
-		"should update the status with a freshly signed certificate only when one doesn't exist": {
+		"should update the status with a freshly signed certificate only when one doesn't exist and group ref=''": {
 			Issuer: gen.Issuer("test",
 				gen.AddIssuerCondition(cmapi.IssuerCondition{
 					Type:   cmapi.IssuerConditionReady,
@@ -260,6 +268,58 @@ func TestSync(t *testing.T) {
 			},
 			Err: false,
 		},
+		"should update the status with a freshly signed certificate only when one doesn't exist and issuer group ref='certmanager.k8s.io'": {
+			Issuer: gen.Issuer("test",
+				gen.AddIssuerCondition(cmapi.IssuerCondition{
+					Type:   cmapi.IssuerConditionReady,
+					Status: cmapi.ConditionTrue,
+				}),
+				gen.SetIssuerSelfSigned(cmapi.SelfSignedIssuer{}),
+			),
+			CertificateRequest: *exampleCRCorrentIssuerRefGroup,
+			IssuerImpl: &fake.Issuer{
+				FakeSign: func(context.Context, *cmapi.CertificateRequest) (*issuer.IssueResponse, error) {
+					return &issuer.IssueResponse{
+						Certificate: certPEM,
+					}, nil
+				},
+			},
+			Builder: &testpkg.Builder{
+				CertManagerObjects: []runtime.Object{gen.CertificateRequest("test")},
+				ExpectedActions: []testpkg.Action{
+					testpkg.NewAction(coretesting.NewUpdateAction(
+						cmapi.SchemeGroupVersion.WithResource("certificaterequests"),
+						gen.DefaultTestNamespace,
+						exampleCRReadyConditionWithGroupRef,
+					)),
+				},
+			},
+			CheckFn: func(t *testing.T, s *controllerFixture, args ...interface{}) {
+			},
+			Err: false,
+		},
+		"should exit sync nil if issuerRef group does not match certmanager.k8s.io": {
+			Issuer: gen.Issuer("test",
+				gen.AddIssuerCondition(cmapi.IssuerCondition{
+					Type:   cmapi.IssuerConditionReady,
+					Status: cmapi.ConditionTrue,
+				}),
+				gen.SetIssuerSelfSigned(cmapi.SelfSignedIssuer{}),
+			),
+			CertificateRequest: *exampleCRWrongIssuerRefGroup,
+			IssuerImpl: &fake.Issuer{
+				FakeSign: func(context.Context, *cmapi.CertificateRequest) (*issuer.IssueResponse, error) {
+					return nil, errors.New("unexpected sign call")
+				},
+			},
+			Builder: &testpkg.Builder{
+				CertManagerObjects: []runtime.Object{gen.CertificateRequest("test")},
+				ExpectedActions:    []testpkg.Action{}, // no update
+			},
+			CheckFn: func(t *testing.T, s *controllerFixture, args ...interface{}) {
+			},
+			Err: false,
+		},
 		"should not update certificate request if certificate exists, even if out of date": {
 			Issuer: gen.Issuer("test",
 				gen.AddIssuerCondition(cmapi.IssuerCondition{

From e41a8f18927e3f4c1b864bae1eee3829e5252fa0 Mon Sep 17 00:00:00 2001
From: JoshVanL <vleeuwenjoshua@gmail.com>
Date: Tue, 9 Jul 2019 13:12:37 +0100
Subject: [PATCH 2/2] Adds not certmanager group ref bailout for certificates
 controller

Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>
---
 pkg/controller/certificates/BUILD.bazel  |  1 +
 pkg/controller/certificates/sync.go      |  7 +++++++
 pkg/controller/certificates/sync_test.go | 23 +++++++++++++++++++++++
 3 files changed, 31 insertions(+)

diff --git a/pkg/controller/certificates/BUILD.bazel b/pkg/controller/certificates/BUILD.bazel
index aec3e0f58..1b78af004 100644
--- a/pkg/controller/certificates/BUILD.bazel
+++ b/pkg/controller/certificates/BUILD.bazel
@@ -11,6 +11,7 @@ go_library(
     visibility = ["//visibility:public"],
     deps = [
         "//pkg/api/util:go_default_library",
+        "//pkg/apis/certmanager:go_default_library",
         "//pkg/apis/certmanager/v1alpha1:go_default_library",
         "//pkg/apis/certmanager/validation:go_default_library",
         "//pkg/client/clientset/versioned:go_default_library",
diff --git a/pkg/controller/certificates/sync.go b/pkg/controller/certificates/sync.go
index 74cc8d353..6fd8e453f 100644
--- a/pkg/controller/certificates/sync.go
+++ b/pkg/controller/certificates/sync.go
@@ -37,6 +37,7 @@ import (
 	"k8s.io/client-go/tools/cache"
 
 	apiutil "github.com/jetstack/cert-manager/pkg/api/util"
+	"github.com/jetstack/cert-manager/pkg/apis/certmanager"
 	"github.com/jetstack/cert-manager/pkg/apis/certmanager/v1alpha1"
 	"github.com/jetstack/cert-manager/pkg/apis/certmanager/validation"
 	"github.com/jetstack/cert-manager/pkg/feature"
@@ -79,6 +80,12 @@ func (c *controller) Sync(ctx context.Context, crt *v1alpha1.Certificate) (err e
 	log := logf.FromContext(ctx)
 	dbg := log.V(logf.DebugLevel)
 
+	// TODO: if not 'certmanager.k8s.io, then use CertificateRequest stratagy if feature gate set
+	if !(crt.Spec.IssuerRef.Group == "" || crt.Spec.IssuerRef.Group == certmanager.GroupName) {
+		dbg.Info("certificate issuerRef group does not match certmanager group so skipping processing")
+		return nil
+	}
+
 	crtCopy := crt.DeepCopy()
 	defer func() {
 		if _, saveErr := c.updateCertificateStatus(ctx, crt, crtCopy); saveErr != nil {
diff --git a/pkg/controller/certificates/sync_test.go b/pkg/controller/certificates/sync_test.go
index a3129d7eb..11072a539 100644
--- a/pkg/controller/certificates/sync_test.go
+++ b/pkg/controller/certificates/sync_test.go
@@ -25,6 +25,7 @@ import (
 	"crypto/x509"
 	"crypto/x509/pkix"
 	"encoding/pem"
+	"errors"
 	"math/big"
 	"testing"
 	"time"
@@ -147,6 +148,9 @@ func TestSync(t *testing.T) {
 
 	localTempCert := generateSelfSignedCert(t, exampleCert, big.NewInt(staticTemporarySerialNumber), pk1, nowTime, nowTime)
 
+	exampleCertWrongGroup := exampleCert.DeepCopy()
+	exampleCertWrongGroup.Spec.IssuerRef.Group = "wrong.group.io"
+
 	tests := map[string]controllerFixture{
 		"should update certificate with NotExists if issuer does not return a keypair": {
 			Issuer: gen.Issuer("test",
@@ -842,6 +846,25 @@ func TestSync(t *testing.T) {
 				},
 			},
 		},
+		"should exit sync nil if group is not certmanager.k8s.io": {
+			Issuer: gen.Issuer("test",
+				gen.AddIssuerCondition(cmapi.IssuerCondition{
+					Type:   cmapi.IssuerConditionReady,
+					Status: cmapi.ConditionTrue,
+				}),
+				gen.SetIssuerSelfSigned(cmapi.SelfSignedIssuer{}),
+			),
+			Certificate: *exampleCertWrongGroup,
+			IssuerImpl: &fake.Issuer{
+				FakeIssue: func(context.Context, *cmapi.Certificate) (*issuer.IssueResponse, error) {
+					return nil, errors.New("unexpected issue call")
+				},
+			},
+			Builder: &testpkg.Builder{
+				CertManagerObjects: []runtime.Object{gen.Certificate("test")},
+				ExpectedActions:    []testpkg.Action{},
+			},
+		},
 		//"should add annotations to already existing secret resource": {
 		//	Issuer: gen.Issuer("test",
 		//		gen.AddIssuerCondition(cmapi.IssuerCondition{