search

Skyscanner / kms-issuer

KMS issuer is a cert-manager Certificate Request controller that uses AWS KMS to sign the certificate request.
Apache License 2.0
62 stars 18 forks source link

KMSKey doesn't validate the configuration on creating which leads to errors when trying to delete it #8

Open danmx opened 4 years ago

danmx commented 4 years ago

AWS only supports deletionPendingWindowInDays from 7 to 30 days. When creating a resource there is no problem. It only raises the issue when you want to delete the key.

Creation:

cat << EOF | kubectl apply -f -
---
apiVersion: cert-manager.skyscanner.net/v1alpha1
kind: KMSKey
metadata:
  name: kmskey-example1
spec:
  aliasName: alias/k8s-certs-kmskey-example1
  description: a kms-issuer example kms key
  customerMasterKeySpec: RSA_2048
  tags:
    Project: k8s
  deletionPolicy: Delete
  deletionPendingWindowInDays: 1
EOF
kmskey.cert-manager.skyscanner.net/kmskey-example1 created

Logs:

 2020-08-07T08:44:29.300Z    ERROR    controllers.kmskey_controller    Failed to delete the KMS key    {"kmskey": "/kmskey-example1", "error": "ValidationException: PendingWindowInDays must be between 7 and 30\n\tstatus code: 400, request
 id: 16f988b6-51eb-46e7-8d7a-6dc92df5309f"}
 github.com/go-logr/zapr.(*zapLogger).Error
     /go/pkg/mod/github.com/go-logr/zapr@v0.1.1/zapr.go:128
 github.com/Skyscanner/kms-issuer/controllers.(*KMSKeyReconciler).manageFailure
     /workspace/controllers/kmskey_controller.go:138
 github.com/Skyscanner/kms-issuer/controllers.(*KMSKeyReconciler).Reconcile
     /workspace/controllers/kmskey_controller.go:88
 sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).reconcileHandler
     /go/pkg/mod/sigs.k8s.io/controller-runtime@v0.5.1-0.20200416234307-5377effd4043/pkg/internal/controller/controller.go:256
 sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).processNextWorkItem
     /go/pkg/mod/sigs.k8s.io/controller-runtime@v0.5.1-0.20200416234307-5377effd4043/pkg/internal/controller/controller.go:232
 sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).worker
     /go/pkg/mod/sigs.k8s.io/controller-runtime@v0.5.1-0.20200416234307-5377effd4043/pkg/internal/controller/controller.go:211
 k8s.io/apimachinery/pkg/util/wait.BackoffUntil.func1
     /go/pkg/mod/k8s.io/apimachinery@v0.18.1/pkg/util/wait/wait.go:155
 k8s.io/apimachinery/pkg/util/wait.BackoffUntil
     /go/pkg/mod/k8s.io/apimachinery@v0.18.1/pkg/util/wait/wait.go:156
 k8s.io/apimachinery/pkg/util/wait.JitterUntil
     /go/pkg/mod/k8s.io/apimachinery@v0.18.1/pkg/util/wait/wait.go:133
 k8s.io/apimachinery/pkg/util/wait.Until
     /go/pkg/mod/k8s.io/apimachinery@v0.18.1/pkg/util/wait/wait.go:90

Mitigation:

Manually edit KMSKey deletionPendingWindowInDays value to correct one and kms-issuer will schedule deletion of the key in AWS

helixphoenix commented 4 years ago

I think we should fix it from the AWS sdk first as they are not checking this input at the creation https://github.com/aws/aws-sdk-go/blob/master/service/kms/api.go

helixphoenix commented 4 years ago

If we want a validation for mechanism, first we should be able to map it at aws sdk, another question is 0 accepted during deletion ? We mabe should not omit empty as well.