Skip to content

Commit

Permalink
do not print warning, when ignoring certs
Browse files Browse the repository at this point in the history
  • Loading branch information
Damian Czaja committed Oct 30, 2024
1 parent 5681f7f commit 658fe2e
Show file tree
Hide file tree
Showing 2 changed files with 130 additions and 58 deletions.
66 changes: 28 additions & 38 deletions internal/actions/csr/csr.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,32 @@ func (c *Certificate) Approved() bool {
return false
}

func (c *Certificate) Outdated() bool {
if c.v1Beta1 != nil {
return c.v1Beta1.CreationTimestamp.Add(csrTTL).Before(time.Now())
}
return c.v1.CreationTimestamp.Add(csrTTL).Before(time.Now())
}

func (c *Certificate) ForCASTAINode() bool {
if c.Name == "" {
return false
}

if strings.HasPrefix(c.Name, "system:node") && strings.Contains(c.Name, "cast-pool") {
return true
}

return false
}

func (c *Certificate) NodeBootstrap() bool {
// Since we only have one handler per CSR/certificate name,
// which is the node name, we can process the controller's certificates and kubelet-bootstrap`s.
// This covers the case when the controller restarts but the bootstrap certificate was deleted without our own certificate being approved.
return c.RequestingUser == "kubelet-bootstrap" || c.RequestingUser == "system:serviceaccount:castai-agent:castai-cluster-controller"
}

func isAlreadyApproved(err error) bool {
if err == nil {
return false
Expand Down Expand Up @@ -338,12 +364,7 @@ func WatchCastAINodeCSRs(ctx context.Context, log logrus.FieldLogger, client kub
return nil
}

var (
errUnexpectedObjectType = errors.New("unexpected object type")
errCSRTooOld = errors.New("csr is too old")
errOwner = errors.New("owner is not bootstrap")
errNonCastAINode = errors.New("not a castai node")
)
var errUnexpectedObjectType = errors.New("unexpected object type")

func processCSREvent(ctx context.Context, c chan<- *Certificate, csrObj interface{}) error {
cert, err := toCertificate(csrObj)
Expand All @@ -355,7 +376,7 @@ func processCSREvent(ctx context.Context, c chan<- *Certificate, csrObj interfac
return nil
}

if cert.Approved() {
if cert.Approved() || !cert.ForCASTAINode() || !cert.NodeBootstrap() || cert.Outdated() {
return nil
}

Expand All @@ -367,58 +388,29 @@ func toCertificate(obj interface{}) (cert *Certificate, err error) {
var name string
var request []byte

isOutdated := false
switch e := obj.(type) {
case *certv1.CertificateSigningRequest:
name = e.Name
request = e.Spec.Request
cert = &Certificate{Name: name, v1: e, RequestingUser: e.Spec.Username}
isOutdated = e.CreationTimestamp.Add(csrTTL).Before(time.Now())
case *certv1beta1.CertificateSigningRequest:
name = e.Name
request = e.Spec.Request
cert = &Certificate{Name: name, v1Beta1: e, RequestingUser: e.Spec.Username}
isOutdated = e.CreationTimestamp.Add(csrTTL).Before(time.Now())
default:
return nil, errUnexpectedObjectType
}

if isOutdated {
return nil, fmt.Errorf("csr with certificate Name: %v RequestingUser: %v %w", cert.Name, cert.RequestingUser, errCSRTooOld)
}

// Since we only have one handler per CSR/certificate name,
// which is the node name, we can process the controller's certificates and kubelet-bootstrap`s.
// This covers the case when the controller restarts but the bootstrap certificate was deleted without our own certificate being approved.
if cert.RequestingUser != "kubelet-bootstrap" && cert.RequestingUser != "system:serviceaccount:castai-agent:castai-cluster-controller" {
return nil, fmt.Errorf("csr with certificate Name: %v RequestingUser: %v %w", cert.Name, cert.RequestingUser, errOwner)
}

cn, err := getSubjectCommonName(name, request)
if err != nil {
return nil, fmt.Errorf("getSubjectCommonName: Name: %v RequestingUser: %v request: %v %w", cert.Name, cert.RequestingUser, string(request), err)
}

if !isCastAINodeCsr(cn) {
return nil, fmt.Errorf("csr with certificate Name: %v RequestingUser: %v cn: %v %w", cert.Name, cert.RequestingUser, cn, errNonCastAINode)
}
cert.Name = cn

return cert, nil
}

func isCastAINodeCsr(subjectCommonName string) bool {
if subjectCommonName == "" {
return false
}

if strings.HasPrefix(subjectCommonName, "system:node") && strings.Contains(subjectCommonName, "cast-pool") {
return true
}

return false
}

func sendCertificate(ctx context.Context, c chan<- *Certificate, cert *Certificate) {
select {
case c <- cert:
Expand Down Expand Up @@ -450,8 +442,6 @@ func parseCSR(pemData []byte) (*x509.CertificateRequest, error) {

//nolint:unparam
func getOptions(signer string) metav1.ListOptions {
fields.SelectorFromSet(fields.Set{})

return metav1.ListOptions{
FieldSelector: fields.SelectorFromSet(fields.Set{
"spec.signerName": signer,
Expand Down
122 changes: 102 additions & 20 deletions internal/actions/csr/csr_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@ package csr
import (
"context"
"path/filepath"
"reflect"
"testing"
"time"

"github.com/stretchr/testify/require"
certv1 "k8s.io/api/certificates/v1"
certv1beta1 "k8s.io/api/certificates/v1beta1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/kubernetes"
_ "k8s.io/client-go/plugin/pkg/client/auth/gcp"
Expand Down Expand Up @@ -93,8 +93,82 @@ func Test_isCastAINodeCsr(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := isCastAINodeCsr(tt.args.subjectCommonName)
require.Equal(t, tt.want, got)
cert := &Certificate{
Name: tt.args.subjectCommonName,
}

require.Equal(t, tt.want, cert.ForCASTAINode())
})
}
}

func Test_outdatedCertificate(t *testing.T) {
tt := map[string]struct {
createTimestamp time.Time
want bool
}{
"Outdated": {
createTimestamp: time.Now().Add(-csrTTL).Add(-time.Second),
want: true,
},
"Not outdated": {
createTimestamp: time.Now(),
want: false,
},
"Outdated, right before": {
createTimestamp: time.Now().Add(-csrTTL).Add(2 * time.Second),
want: false,
},
}

for name, tc := range tt {
t.Run(name, func(t *testing.T) {
cert := &Certificate{
v1: &certv1.CertificateSigningRequest{
ObjectMeta: metav1.ObjectMeta{
CreationTimestamp: metav1.NewTime(tc.createTimestamp),
},
},
}
require.Equal(t, tc.want, cert.Outdated())

certBeta := &Certificate{
v1Beta1: &certv1beta1.CertificateSigningRequest{
ObjectMeta: metav1.ObjectMeta{
CreationTimestamp: metav1.NewTime(tc.createTimestamp),
},
},
}
require.Equal(t, tc.want, certBeta.Outdated())
})
}
}

func Test_nodeBootstrap(t *testing.T) {
tt := map[string]struct {
reqUser string
want bool
}{
"other one": {
reqUser: "dummy-user",
want: false,
},
"kubelet-bootstrap": {
reqUser: "kubelet-bootstrap",
want: true,
},
"castai-cluster-controller": {
reqUser: "system:serviceaccount:castai-agent:castai-cluster-controller",
want: true,
},
}

for name, tc := range tt {
t.Run(name, func(t *testing.T) {
cert := &Certificate{
RequestingUser: tc.reqUser,
}
require.Equal(t, tc.want, cert.NodeBootstrap())
})
}
}
Expand All @@ -106,10 +180,10 @@ func Test_toCertificate(t *testing.T) {
obj interface{}
}
tests := []struct {
name string
args args
wantCert *Certificate
wantErr bool
name string
args args
checkFunc func(t *testing.T, cert *Certificate)
wantErr bool
}{
{
name: "empty event",
Expand All @@ -125,9 +199,13 @@ func Test_toCertificate(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{
CreationTimestamp: metav1.Time{Time: time.Now().Add(-csrTTL)},
},
Spec: testCSRv1.Spec,
},
},
wantErr: true,
checkFunc: func(t *testing.T, cert *Certificate) {
require.True(t, cert.Outdated())
},
wantErr: false,
},
{
name: "bad owner",
Expand All @@ -141,30 +219,33 @@ func Test_toCertificate(t *testing.T) {
},
},
},
wantErr: true,
checkFunc: func(t *testing.T, cert *Certificate) {
require.False(t, cert.NodeBootstrap())
},
wantErr: false,
},
{
name: "ok v1",
args: args{
obj: testCSRv1,
},
wantErr: false,
wantCert: &Certificate{
Name: "system:node:gke-dev-master-cast-pool-cb53177b",
RequestingUser: "kubelet-bootstrap",
v1: testCSRv1,
checkFunc: func(t *testing.T, cert *Certificate) {
require.Equal(t, "system:node:gke-dev-master-cast-pool-cb53177b", cert.Name)
require.Equal(t, "kubelet-bootstrap", cert.RequestingUser)
require.Equal(t, testCSRv1, cert.v1)
},
wantErr: false,
},
{
name: "ok v1beta1",
args: args{
obj: testCSRv1beta1,
},
wantErr: false,
wantCert: &Certificate{
Name: "system:node:gke-dev-master-cast-pool-cb53177b",
RequestingUser: "kubelet-bootstrap",
v1Beta1: testCSRv1beta1,
checkFunc: func(t *testing.T, cert *Certificate) {
require.Equal(t, "system:node:gke-dev-master-cast-pool-cb53177b", cert.Name)
require.Equal(t, "kubelet-bootstrap", cert.RequestingUser)
require.Equal(t, testCSRv1beta1, cert.v1Beta1)
},
},
}
Expand All @@ -175,8 +256,9 @@ func Test_toCertificate(t *testing.T) {
t.Errorf("toCertificate() error = %v, wantErr %v", err, tt.wantErr)
return
}
if !reflect.DeepEqual(gotCert, tt.wantCert) {
t.Errorf("toCertificate() gotCert = %v, want %v", gotCert, tt.wantCert)

if tt.checkFunc != nil {
tt.checkFunc(t, gotCert)
}
})
}
Expand Down

0 comments on commit 658fe2e

Please sign in to comment.