Make signer admission plugin check on condition update

This commit is contained in:
Jordan Liggitt 2020-04-16 00:14:29 -04:00
parent aed0621f2e
commit 377adfa2b7
2 changed files with 28 additions and 7 deletions

View File

@ -24,10 +24,10 @@ import (
"k8s.io/klog/v2"
apiequality "k8s.io/apimachinery/pkg/api/equality"
"k8s.io/apiserver/pkg/admission"
genericadmissioninit "k8s.io/apiserver/pkg/admission/initializer"
"k8s.io/apiserver/pkg/authorization/authorizer"
api "k8s.io/kubernetes/pkg/apis/certificates"
"k8s.io/kubernetes/plugin/pkg/admission/certificates"
)
@ -73,10 +73,10 @@ func NewPlugin() *Plugin {
var csrGroupResource = api.Resource("certificatesigningrequests")
// Validate verifies that the requesting user has permission to approve
// Validate verifies that the requesting user has permission to sign
// CertificateSigningRequests for the specified signerName.
func (p *Plugin) Validate(ctx context.Context, a admission.Attributes, o admission.ObjectInterfaces) error {
// Ignore all calls to anything other than 'certificatesigningrequests/approval'.
// Ignore all calls to anything other than 'certificatesigningrequests/status'.
// Ignore all operations other than UPDATE.
if a.GetSubresource() != "status" ||
a.GetResource().GroupResource() != csrGroupResource {
@ -92,8 +92,8 @@ func (p *Plugin) Validate(ctx context.Context, a admission.Attributes, o admissi
return admission.NewForbidden(a, fmt.Errorf("expected type CertificateSigningRequest, got: %T", a.GetObject()))
}
// only run if the status.certificate field has been changed
if reflect.DeepEqual(oldCSR.Status.Certificate, csr.Status.Certificate) {
// only run if the status.certificate or status.conditions field has been changed
if reflect.DeepEqual(oldCSR.Status.Certificate, csr.Status.Certificate) && apiequality.Semantic.DeepEqual(oldCSR.Status.Conditions, csr.Status.Conditions) {
return nil
}

View File

@ -48,7 +48,7 @@ func TestPlugin_Validate(t *testing.T) {
},
allowed: false,
},
"allowed if the 'certificate' field has not changed": {
"allowed if the 'certificate' and conditions field has not changed": {
attributes: &testAttributes{
resource: certificatesapi.Resource("certificatesigningrequests"),
subresource: "status",
@ -63,7 +63,7 @@ func TestPlugin_Validate(t *testing.T) {
allowed: true,
authzErr: errors.New("faked error"),
},
"deny request if authz lookup fails": {
"deny request if authz lookup fails on certificate change": {
allowedName: "abc.com/xyz",
attributes: &testAttributes{
resource: certificatesapi.Resource("certificatesigningrequests"),
@ -84,6 +84,27 @@ func TestPlugin_Validate(t *testing.T) {
authzErr: errors.New("test"),
allowed: false,
},
"deny request if authz lookup fails on condition change": {
allowedName: "abc.com/xyz",
attributes: &testAttributes{
resource: certificatesapi.Resource("certificatesigningrequests"),
subresource: "status",
oldObj: &certificatesapi.CertificateSigningRequest{Spec: certificatesapi.CertificateSigningRequestSpec{
SignerName: "abc.com/xyz",
}},
obj: &certificatesapi.CertificateSigningRequest{
Spec: certificatesapi.CertificateSigningRequestSpec{
SignerName: "abc.com/xyz",
},
Status: certificatesapi.CertificateSigningRequestStatus{
Conditions: []certificatesapi.CertificateSigningRequestCondition{{Type: certificatesapi.CertificateFailed}},
},
},
operation: admission.Update,
},
authzErr: errors.New("test"),
allowed: false,
},
"allow request if user is authorized for specific signerName": {
allowedName: "abc.com/xyz",
attributes: &testAttributes{