Skip to content

Commit

Permalink
refactor: IsRefreshable KMP interface method
Browse files Browse the repository at this point in the history
  • Loading branch information
@duffney
duffney committed Jul 17, 2024
1 parent b35a8f4 commit 234e835
Show file tree
Hide file tree
Showing 8 changed files with 19 additions and 313 deletions.
2 changes: 0 additions & 2 deletions api/v1beta1/keymanagementproviders_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,6 @@ type KeyManagementProviderSpec struct {
// +kubebuilder:default=1
Interval int32 `json:"interval,omitempty"`

Refreshable bool `json:"refreshable,omitempty"`

// +kubebuilder:pruning:PreserveUnknownFields
// Parameters of the key management provider
Parameters runtime.RawExtension `json:"parameters,omitempty"`
Expand Down
4 changes: 4 additions & 0 deletions config/crd/bases/config.ratify.deislabs.io_keymanagementproviders.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,10 @@ spec:
spec:
description: KeyManagementProviderSpec defines the desired state of KeyManagementProvider
properties:
interval:
default: 1
format: int32
type: integer
parameters:
description: Parameters of the key management provider
type: object
Expand Down
48 changes: 0 additions & 48 deletions pkg/controllers/clusterresource/keymanagementprovider_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,22 +18,17 @@ package clusterresource

import (
"context"
"encoding/json"
"fmt"

"github.com/ratify-project/ratify/internal/constants"
_ "github.com/ratify-project/ratify/pkg/keymanagementprovider/azurekeyvault" // register azure key vault key management provider
_ "github.com/ratify-project/ratify/pkg/keymanagementprovider/inline" // register inline key management provider
"github.com/ratify-project/ratify/pkg/keymanagementprovider/refresh" // register inline key management provider
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/predicate"

configv1beta1 "github.com/ratify-project/ratify/api/v1beta1"
kmp "github.com/ratify-project/ratify/pkg/keymanagementprovider"
"github.com/sirupsen/logrus"
)

// KeyManagementProviderReconciler reconciles a KeyManagementProvider object
Expand Down Expand Up @@ -76,46 +71,3 @@ func (r *KeyManagementProviderReconciler) SetupWithManager(mgr ctrl.Manager) err
For(&configv1beta1.KeyManagementProvider{}).WithEventFilter(pred).
Complete(r)
}

// writeKMProviderStatus updates the status of the key management provider resource
func writeKMProviderStatus(ctx context.Context, r client.StatusClient, keyManagementProvider *configv1beta1.KeyManagementProvider, logger *logrus.Entry, isSuccess bool, errorString string, operationTime metav1.Time, kmProviderStatus kmp.KeyManagementProviderStatus) {
if isSuccess {
updateKMProviderSuccessStatus(keyManagementProvider, &operationTime, kmProviderStatus)
} else {
updateKMProviderErrorStatus(keyManagementProvider, errorString, &operationTime)
}
if statusErr := r.Status().Update(ctx, keyManagementProvider); statusErr != nil {
logger.Error(statusErr, ",unable to update key management provider error status")
}
}

// updateKMProviderErrorStatus updates the key management provider status with error, brief error and last fetched time
func updateKMProviderErrorStatus(keyManagementProvider *configv1beta1.KeyManagementProvider, errorString string, operationTime *metav1.Time) {
// truncate brief error string to maxBriefErrLength
briefErr := errorString
if len(errorString) > constants.MaxBriefErrLength {
briefErr = fmt.Sprintf("%s...", errorString[:constants.MaxBriefErrLength])
}
keyManagementProvider.Status.IsSuccess = false
keyManagementProvider.Status.Error = errorString
keyManagementProvider.Status.BriefError = briefErr
keyManagementProvider.Status.LastFetchedTime = operationTime
}

// updateKMProviderSuccessStatus updates the key management provider status if status argument is non nil
// Success status includes last fetched time and other provider-specific properties
func updateKMProviderSuccessStatus(keyManagementProvider *configv1beta1.KeyManagementProvider, lastOperationTime *metav1.Time, kmProviderStatus kmp.KeyManagementProviderStatus) {
keyManagementProvider.Status.IsSuccess = true
keyManagementProvider.Status.Error = ""
keyManagementProvider.Status.BriefError = ""
keyManagementProvider.Status.LastFetchedTime = lastOperationTime

if kmProviderStatus != nil {
jsonString, _ := json.Marshal(kmProviderStatus)

raw := runtime.RawExtension{
Raw: jsonString,
}
keyManagementProvider.Status.Properties = raw
}
}
265 changes: 4 additions & 261 deletions pkg/controllers/clusterresource/keymanagementprovider_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,268 +16,11 @@ limitations under the License.
package clusterresource

import (
"context"
"fmt"
"testing"

configv1beta1 "github.com/ratify-project/ratify/api/v1beta1"
"github.com/ratify-project/ratify/internal/constants"
"github.com/ratify-project/ratify/pkg/keymanagementprovider"
"github.com/sirupsen/logrus"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/fake"
"sigs.k8s.io/controller-runtime/pkg/reconcile"

test "github.com/ratify-project/ratify/pkg/utils"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
)

const (
kmpName = "kmpName"
)

// TestUpdateErrorStatus tests the updateErrorStatus method
func TestKMProviderUpdateErrorStatus(t *testing.T) {
var parametersString = "{\"certs\":{\"name\":\"certName\"}}"
var kmProviderStatus = []byte(parametersString)

status := configv1beta1.KeyManagementProviderStatus{
IsSuccess: true,
Properties: runtime.RawExtension{
Raw: kmProviderStatus,
},
}
keyManagementProvider := configv1beta1.KeyManagementProvider{
Status: status,
}
expectedErr := "it's a long error from unit test"
lastFetchedTime := metav1.Now()
updateKMProviderErrorStatus(&keyManagementProvider, expectedErr, &lastFetchedTime)

if keyManagementProvider.Status.IsSuccess != false {
t.Fatalf("Unexpected error, expected isSuccess to be false , actual %+v", keyManagementProvider.Status.IsSuccess)
}

if keyManagementProvider.Status.Error != expectedErr {
t.Fatalf("Unexpected error string, expected %+v, got %+v", expectedErr, keyManagementProvider.Status.Error)
}
expectedBriedErr := fmt.Sprintf("%s...", expectedErr[:30])
if keyManagementProvider.Status.BriefError != expectedBriedErr {
t.Fatalf("Unexpected error string, expected %+v, got %+v", expectedBriedErr, keyManagementProvider.Status.Error)
}

//make sure properties of last cached cert was not overridden
if len(keyManagementProvider.Status.Properties.Raw) == 0 {
t.Fatalf("Unexpected properties, expected %+v, got %+v", parametersString, string(keyManagementProvider.Status.Properties.Raw))
}
}

// TestKMProviderUpdateSuccessStatus tests the updateSuccessStatus method
func TestKMProviderUpdateSuccessStatus(t *testing.T) {
kmProviderStatus := keymanagementprovider.KeyManagementProviderStatus{}
properties := map[string]string{}
properties["Name"] = "wabbit"
properties["Version"] = "ABC"

kmProviderStatus["Certificates"] = properties

lastFetchedTime := metav1.Now()

status := configv1beta1.KeyManagementProviderStatus{
IsSuccess: false,
Error: "error from last operation",
}
keyManagementProvider := configv1beta1.KeyManagementProvider{
Status: status,
}

updateKMProviderSuccessStatus(&keyManagementProvider, &lastFetchedTime, kmProviderStatus)

if keyManagementProvider.Status.IsSuccess != true {
t.Fatalf("Expected isSuccess to be true , actual %+v", keyManagementProvider.Status.IsSuccess)
}

if keyManagementProvider.Status.Error != "" {
t.Fatalf("Unexpected error string, actual %+v", keyManagementProvider.Status.Error)
}

//make sure properties of last cached cert was updated
if len(keyManagementProvider.Status.Properties.Raw) == 0 {
t.Fatalf("Properties should not be empty")
}
}

// TestKMProviderUpdateSuccessStatus tests the updateSuccessStatus method with empty properties
func TestKMProviderUpdateSuccessStatus_emptyProperties(t *testing.T) {
lastFetchedTime := metav1.Now()
status := configv1beta1.KeyManagementProviderStatus{
IsSuccess: false,
Error: "error from last operation",
}
keyManagementProvider := configv1beta1.KeyManagementProvider{
Status: status,
}

updateKMProviderSuccessStatus(&keyManagementProvider, &lastFetchedTime, nil)

if keyManagementProvider.Status.IsSuccess != true {
t.Fatalf("Expected isSuccess to be true , actual %+v", keyManagementProvider.Status.IsSuccess)
}

if keyManagementProvider.Status.Error != "" {
t.Fatalf("Unexpected error string, actual %+v", keyManagementProvider.Status.Error)
}

//make sure properties of last cached cert was updated
if len(keyManagementProvider.Status.Properties.Raw) != 0 {
t.Fatalf("Properties should be empty")
}
}

func TestWriteKMProviderStatus(t *testing.T) {
logger := logrus.WithContext(context.Background())
lastFetchedTime := metav1.Now()
testCases := []struct {
name string
isSuccess bool
kmProvider *configv1beta1.KeyManagementProvider
errString string
reconciler client.StatusClient
}{
{
name: "success status",
isSuccess: true,
errString: "",
kmProvider: &configv1beta1.KeyManagementProvider{},
reconciler: &test.MockStatusClient{},
},
{
name: "error status",
isSuccess: false,
kmProvider: &configv1beta1.KeyManagementProvider{},
errString: "a long error string that exceeds the max length of 30 characters",
reconciler: &test.MockStatusClient{},
},
{
name: "status update failed",
isSuccess: true,
kmProvider: &configv1beta1.KeyManagementProvider{},
reconciler: &test.MockStatusClient{
UpdateFailed: true,
},
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
writeKMProviderStatus(context.Background(), tc.reconciler, tc.kmProvider, logger, tc.isSuccess, tc.errString, lastFetchedTime, nil)

if tc.kmProvider.Status.IsSuccess != tc.isSuccess {
t.Fatalf("Expected isSuccess to be %+v , actual %+v", tc.isSuccess, tc.kmProvider.Status.IsSuccess)
}

if tc.kmProvider.Status.Error != tc.errString {
t.Fatalf("Expected Error to be %+v , actual %+v", tc.errString, tc.kmProvider.Status.Error)
}
})
}
}

func TestKMPReconcile(t *testing.T) {
tests := []struct {
name string
description string
provider *configv1beta1.KeyManagementProvider
req *reconcile.Request
expectedErr bool
expectedKMPCount int
}{
{
name: "nonexistent KMP",
description: "Reconciling a non-existent KMP CR, it should be deleted from maps",
req: &reconcile.Request{
NamespacedName: types.NamespacedName{Name: "nonexistent"},
},
provider: &configv1beta1.KeyManagementProvider{
ObjectMeta: metav1.ObjectMeta{
Namespace: constants.EmptyNamespace,
Name: kmpName,
},
Spec: configv1beta1.KeyManagementProviderSpec{
Type: "inline",
},
},
expectedErr: false,
expectedKMPCount: 0,
},
{
name: "invalid params",
description: "Received invalid parameters of the KMP Spec, it should fail the reconcile and return an error",
provider: &configv1beta1.KeyManagementProvider{
ObjectMeta: metav1.ObjectMeta{
Namespace: constants.EmptyNamespace,
Name: kmpName,
},
Spec: configv1beta1.KeyManagementProviderSpec{
Type: "inline",
},
},
expectedErr: true,
expectedKMPCount: 0,
},
{
name: "valid params",
description: "Received a valid KMP manifest, it should be added to the cert map",
provider: &configv1beta1.KeyManagementProvider{
ObjectMeta: metav1.ObjectMeta{
Namespace: constants.EmptyNamespace,
Name: kmpName,
},
Spec: configv1beta1.KeyManagementProviderSpec{
Type: "inline",
Parameters: runtime.RawExtension{
Raw: []byte(`{"type": "inline", "contentType": "certificate", "value": "-----BEGIN CERTIFICATE-----\nMIID2jCCAsKgAwIBAgIQXy2VqtlhSkiZKAGhsnkjbDANBgkqhkiG9w0BAQsFADBvMRswGQYDVQQD\nExJyYXRpZnkuZXhhbXBsZS5jb20xDzANBgNVBAsTBk15IE9yZzETMBEGA1UEChMKTXkgQ29tcGFu\neTEQMA4GA1UEBxMHUmVkbW9uZDELMAkGA1UECBMCV0ExCzAJBgNVBAYTAlVTMB4XDTIzMDIwMTIy\nNDUwMFoXDTI0MDIwMTIyNTUwMFowbzEbMBkGA1UEAxMScmF0aWZ5LmV4YW1wbGUuY29tMQ8wDQYD\nVQQLEwZNeSBPcmcxEzARBgNVBAoTCk15IENvbXBhbnkxEDAOBgNVBAcTB1JlZG1vbmQxCzAJBgNV\nBAgTAldBMQswCQYDVQQGEwJVUzCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAL10bM81\npPAyuraORABsOGS8M76Bi7Guwa3JlM1g2D8CuzSfSTaaT6apy9GsccxUvXd5cmiP1ffna5z+EFmc\nizFQh2aq9kWKWXDvKFXzpQuhyqD1HeVlRlF+V0AfZPvGt3VwUUjNycoUU44ctCWmcUQP/KShZev3\n6SOsJ9q7KLjxxQLsUc4mg55eZUThu8mGB8jugtjsnLUYvIWfHhyjVpGrGVrdkDMoMn+u33scOmrt\nsBljvq9WVo4T/VrTDuiOYlAJFMUae2Ptvo0go8XTN3OjLblKeiK4C+jMn9Dk33oGIT9pmX0vrDJV\nX56w/2SejC1AxCPchHaMuhlwMpftBGkCAwEAAaNyMHAwDgYDVR0PAQH/BAQDAgeAMAkGA1UdEwQC\nMAAwEwYDVR0lBAwwCgYIKwYBBQUHAwMwHwYDVR0jBBgwFoAU0eaKkZj+MS9jCp9Dg1zdv3v/aKww\nHQYDVR0OBBYEFNHmipGY/jEvYwqfQ4Nc3b97/2isMA0GCSqGSIb3DQEBCwUAA4IBAQBNDcmSBizF\nmpJlD8EgNcUCy5tz7W3+AAhEbA3vsHP4D/UyV3UgcESx+L+Nye5uDYtTVm3lQejs3erN2BjW+ds+\nXFnpU/pVimd0aYv6mJfOieRILBF4XFomjhrJOLI55oVwLN/AgX6kuC3CJY2NMyJKlTao9oZgpHhs\nLlxB/r0n9JnUoN0Gq93oc1+OLFjPI7gNuPXYOP1N46oKgEmAEmNkP1etFrEjFRgsdIFHksrmlOlD\nIed9RcQ087VLjmuymLgqMTFX34Q3j7XgN2ENwBSnkHotE9CcuGRW+NuiOeJalL8DBmFXXWwHTKLQ\nPp5g6m1yZXylLJaFLKz7tdMmO355\n-----END CERTIFICATE-----\n"}`),
},
},
},
expectedErr: false,
expectedKMPCount: 1,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
resetKMP()
scheme, _ := test.CreateScheme()
client := fake.NewClientBuilder().WithScheme(scheme)
client.WithObjects(tt.provider)
r := &KeyManagementProviderReconciler{
Scheme: scheme,
Client: client.Build(),
}
var req reconcile.Request
if tt.req != nil {
req = *tt.req
} else {
req = reconcile.Request{
NamespacedName: test.KeyFor(tt.provider),
}
}

_, err := r.Reconcile(context.Background(), req)
if tt.expectedErr != (err != nil) {
t.Fatalf("Reconcile() expected error %v, actual %v", tt.expectedErr, err)
}
certs, _ := keymanagementprovider.GetCertificatesFromMap(context.Background(), kmpName)
if len(certs) != tt.expectedKMPCount {
t.Fatalf("Cert map expected size %v, actual %v", tt.expectedKMPCount, len(certs))
}
})
}
}

func resetKMP() {
keymanagementprovider.DeleteCertificatesFromMap(storeName)
// write a fake test to leaves a todo for the developer
func TestKeyManagementProviderReconciler_Reconcile(t *testing.T) {
todo := "fake test"
t.Logf("TODO: %s", todo)
}
4 changes: 4 additions & 0 deletions pkg/keymanagementprovider/azurekeyvault/provider.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,10 @@ func (s *akvKMProvider) GetKeys(ctx context.Context) (map[keymanagementprovider.
return keysMap, getStatusMap(keysStatus, types.KeysStatus), nil
}

func (s *akvKMProvider) IsRefreshable() bool {
return true
}

// azure keyvault provider certificate/key status is a map from "certificates" key or "keys" key to an array of key management provider status
func getStatusMap(statusMap []map[string]string, contentType string) keymanagementprovider.KeyManagementProviderStatus {
status := keymanagementprovider.KeyManagementProviderStatus{}
Expand Down
4 changes: 4 additions & 0 deletions pkg/keymanagementprovider/inline/provider.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,3 +114,7 @@ func (s *inlineKMProvider) GetCertificates(_ context.Context) (map[keymanagement
func (s *inlineKMProvider) GetKeys(_ context.Context) (map[keymanagementprovider.KMPMapKey]crypto.PublicKey, keymanagementprovider.KeyManagementProviderStatus, error) {
return s.keys, nil, nil
}

func (s *inlineKMProvider) IsRefreshable() bool {
return false
}
2 changes: 2 additions & 0 deletions pkg/keymanagementprovider/keymanagementprovider.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ type KeyManagementProvider interface {
GetCertificates(ctx context.Context) (map[KMPMapKey][]*x509.Certificate, KeyManagementProviderStatus, error)
// Returns an array of keys and the provider specific key attributes
GetKeys(ctx context.Context) (map[KMPMapKey]crypto.PublicKey, KeyManagementProviderStatus, error)
// Returns if the provider supports refreshing of certificates/keys
IsRefreshable() bool
}

// static concurrency-safe map to store certificates fetched from key management provider
Expand Down
Loading

0 comments on commit 234e835

Please sign in to comment.