Merge pull request #514 from jamesglennan/506_usernamCELValidation

Add CertificateRequest username to CEL Validator with serviceaccount functions

go.mod

@@ -17,6 +17,7 @@ require (
 	k8s.io/api v0.31.1
 	k8s.io/apiextensions-apiserver v0.31.1
 	k8s.io/apimachinery v0.31.1
+	k8s.io/apiserver v0.31.1
 	k8s.io/cli-runtime v0.31.1
 	k8s.io/client-go v0.31.1
 	k8s.io/component-base v0.31.1
@@ -111,7 +112,6 @@ require (
 	gopkg.in/inf.v0 v0.9.1 // indirect
 	gopkg.in/yaml.v2 v2.4.0 // indirect
 	gopkg.in/yaml.v3 v3.0.1 // indirect
-	k8s.io/apiserver v0.31.1 // indirect
 	k8s.io/kube-openapi v0.0.0-20240903163716-9e1beecbcb38 // indirect
 	sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.30.3 // indirect
 	sigs.k8s.io/gateway-api v1.1.0 // indirect

pkg/internal/approver/validation/certificaterequest.proto

@@ -7,4 +7,5 @@ option go_package = "github.com/cert-manager/approver-policy/pkg/internal/approv
 message CertificateRequest {
   string name = 1;
   string namespace = 2;
+  string username = 3;
 }

pkg/internal/approver/validation/serviceaccount.go

@@ -0,0 +1,160 @@
+/*
+Copyright 2024 The cert-manager 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 validation
+
+import (
+	"fmt"
+	"reflect"
+
+	"github.com/google/cel-go/cel"
+	"github.com/google/cel-go/common/types"
+	"github.com/google/cel-go/common/types/ref"
+	"k8s.io/apiserver/pkg/authentication/serviceaccount"
+)
+
+var (
+	SAType = cel.ObjectType("cm.io.policy.pkg.internal.approver.validation.ServiceAccount")
+)
+
+type saLib struct{}
+type ServiceAccount struct {
+	Name      string
+	Namespace string
+}
+
+func ServiceAccountLib() cel.EnvOption {
+	return cel.Lib(&saLib{})
+}
+
+// ConvertToNative implements ref.Val.ConvertToNative.
+func (sa ServiceAccount) ConvertToNative(typeDesc reflect.Type) (interface{}, error) {
+	if reflect.TypeOf(sa).AssignableTo(typeDesc) {
+		return sa, nil
+	}
+	if reflect.TypeOf("").AssignableTo(typeDesc) {
+		return serviceaccount.MakeUsername(sa.Namespace, sa.Name), nil
+	}
+	return nil, fmt.Errorf("type conversion error from 'serviceaccount' to '%v'", typeDesc)
+}
+
+// ConvertToType implements ref.Val.ConvertToType.
+func (sa ServiceAccount) ConvertToType(typeVal ref.Type) ref.Val {
+	switch typeVal {
+	case SAType:
+		return sa
+	case types.TypeType:
+		return SAType
+	}
+	return types.NewErr("type conversion error from '%s' to '%s'", SAType, typeVal)
+}
+
+// Equal implements ref.Val.Equal.
+func (sa ServiceAccount) Equal(other ref.Val) ref.Val {
+	otherSA, ok := other.(ServiceAccount)
+	if !ok {
+		return types.MaybeNoSuchOverloadErr(other)
+	}
+	return types.Bool(sa.Name == otherSA.Name && sa.Namespace == otherSA.Namespace)
+}
+
+// Type implements ref.Val.Type.Y
+func (sa ServiceAccount) Type() ref.Type {
+	return SAType
+}
+
+// Value implements ref.Val.Value.
+func (sa ServiceAccount) Value() interface{} {
+	return sa
+}
+
+var saLibraryDecls = map[string][]cel.FunctionOpt{
+	"serviceAccount": {
+		cel.Overload("username_to_serviceaccount", []*cel.Type{cel.StringType}, SAType,
+			cel.UnaryBinding(stringToServiceAccount))},
+	"getName": {
+		cel.MemberOverload("serviceaccount_get_name", []*cel.Type{SAType}, cel.StringType,
+			cel.UnaryBinding(getServiceAccountName))},
+	"getNamespace": {
+		cel.MemberOverload("serviceaccount_get_namespace", []*cel.Type{SAType}, cel.StringType,
+			cel.UnaryBinding(getServiceAccountNamespace))},
+	"isServiceAccount": {
+		cel.Overload("serviceaccount_is_sa", []*cel.Type{cel.StringType}, cel.BoolType,
+			cel.UnaryBinding(isServiceAccount))},
+}
+
+func stringToServiceAccount(arg ref.Val) ref.Val {
+	s, ok := arg.Value().(string)
+	if !ok {
+		return types.MaybeNoSuchOverloadErr(arg)
+	}
+
+	ns, name, err := serviceaccount.SplitUsername(s)
+
+	if err != nil {
+		return types.NewErr("Unable to convert to serviceaccount: err: %s, username: %s", err, s)
+	}
+
+	return ServiceAccount{
+		Name:      name,
+		Namespace: ns,
+	}
+}
+
+func isServiceAccount(arg ref.Val) ref.Val {
+	s, ok := arg.Value().(string)
+	if !ok {
+		return types.MaybeNoSuchOverloadErr(arg)
+	}
+
+	_, _, err := serviceaccount.SplitUsername(s)
+
+	if err != nil {
+		return types.False
+	}
+
+	return types.True
+}
+
+func getServiceAccountName(arg ref.Val) ref.Val {
+	s, ok := arg.Value().(ServiceAccount)
+	if !ok {
+		return types.MaybeNoSuchOverloadErr(arg)
+	}
+
+	return types.String(s.Name)
+}
+
+func getServiceAccountNamespace(arg ref.Val) ref.Val {
+	s, ok := arg.Value().(ServiceAccount)
+	if !ok {
+		return types.MaybeNoSuchOverloadErr(arg)
+	}
+
+	return types.String(s.Namespace)
+}
+
+func (*saLib) CompileOptions() []cel.EnvOption {
+	options := []cel.EnvOption{}
+	for name, overloads := range saLibraryDecls {
+		options = append(options, cel.Function(name, overloads...))
+	}
+	return options
+}
+
+func (*saLib) ProgramOptions() []cel.ProgramOption {
+	return []cel.ProgramOption{}
+}

pkg/internal/approver/validation/validator.go

@@ -61,7 +61,9 @@ func (v *validator) compile() error {
 		cel.Variable(varSelf, cel.StringType),
 		cel.Variable(varRequest, cel.ObjectType("cm.io.policy.pkg.internal.approver.validation.CertificateRequest")),
 		ext.Strings(),
+		ServiceAccountLib(),
 	)
+
 	if err != nil {
 		return err
 	}
@@ -89,6 +91,7 @@ func (v *validator) Validate(value string, request cmapi.CertificateRequest) (bo
 		varRequest: &CertificateRequest{
 			Name:      request.GetName(),
 			Namespace: request.GetNamespace(),
+			Username:  request.Spec.Username,
 		},
 	}
 

pkg/internal/approver/validation/validator_test.go

@@ -38,6 +38,10 @@ func Test_Validator_Compile(t *testing.T) {
 		{name: "err-undeclared-vars", expr: "foo = bar", wantErr: true},
 		{name: "err-must-return-bool", expr: "size('foo')", wantErr: true},
 		{name: "err-invalid-property", expr: "size(cr.foo) < 24", wantErr: true},
+		{name: "check-username-property", expr: "size(cr.username) > 0", wantErr: false},
+		{name: "check-serviceaccount-getname", expr: "self.startsWith(serviceAccount(cr.username).getName())", wantErr: false},
+		{name: "check-serviceaccount-getnamespace", expr: "self.startsWith(serviceAccount(cr.username).getNamespace())", wantErr: false},
+		{name: "check-serviceaccount-isSA", expr: "isServiceAccount(cr.username)", wantErr: false},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -91,3 +95,48 @@ func newCertificateRequest(namespace string) cmapi.CertificateRequest {
 	request.SetNamespace(namespace)
 	return request
 }
+
+func Test_Validator_Validate_ServiceAccount(t *testing.T) {
+	v := &validator{expression: "isServiceAccount(cr.username) && self.startsWith('spiffe://acme.com/ns/%s/sa/%s'.format([serviceAccount(cr.username).getNamespace(),serviceAccount(cr.username).getName()]))"}
+	err := v.compile()
+	assert.NoError(t, err)
+
+	type args struct {
+		val string
+		cr  cmapi.CertificateRequest
+	}
+	tests := []struct {
+		name    string
+		args    args
+		want    bool
+		wantErr bool
+	}{
+		{name: "correct-namespace-and-name", args: args{val: "spiffe://acme.com/ns/foo-ns/sa/bar", cr: newCertificateRequestWithUsername("system:serviceaccount:foo-ns:bar")}, want: true},
+		{name: "correct-namespace-and-name2", args: args{val: "spiffe://acme.com/ns/bar-ns/sa/foo", cr: newCertificateRequestWithUsername("system:serviceaccount:bar-ns:foo")}, want: true},
+		{name: "correct-namespace-wrong-name", args: args{val: "spiffe://acme.com/ns/foo-ns/sa/foo", cr: newCertificateRequestWithUsername("system:serviceaccount:foo-ns:bar")}, want: false},
+		{name: "wrong-namespace-correct-name", args: args{val: "spiffe://acme.com/ns/foo-ns/sa/bar", cr: newCertificateRequestWithUsername("system:serviceaccount:bar-ns:bar")}, want: false},
+		{name: "not-serviceaccount", args: args{val: "spiffe://acme.com/ns/foo-ns/sa/bar", cr: newCertificateRequestWithUsername("bar")}, want: false},
+		{name: "unrelated", args: args{val: "spiffe://example.com", cr: newCertificateRequestWithUsername("system:serviceaccount:foo-ns:bar")}, want: false},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := v.Validate(tt.args.val, tt.args.cr)
+			if tt.wantErr {
+				assert.Error(t, err)
+				return
+			} else {
+				assert.NoError(t, err)
+			}
+			assert.Equal(t, tt.want, got)
+		})
+	}
+}
+
+func newCertificateRequestWithUsername(username string) cmapi.CertificateRequest {
+	request := cmapi.CertificateRequest{
+		Spec: cmapi.CertificateRequestSpec{
+			Username: username,
+		},
+	}
+	return request
+}