Merge pull request #1860 from JoshVanL/cr-group-ref

Include Group name in IssuerRef for CertificateRequest controller ownership distinction
This commit is contained in:
jetstack-bot 2019-07-09 14:10:04 +01:00 committed by GitHub
commit 13ebd873d9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 123 additions and 7 deletions

View File

@ -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:

View File

@ -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>

View File

@ -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 (

View File

@ -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

View File

@ -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",

View File

@ -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,

View File

@ -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{

View File

@ -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",

View File

@ -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 {

View File

@ -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{