fix: remove disabled certs from cache

Signed-off-by: Joshua Duffney <[email protected]>

pkg/controllers/clusterresource/keymanagementprovider_controller.go

@@ -80,7 +80,7 @@ func (r *KeyManagementProviderReconciler) ReconcileWithType(ctx context.Context,
 		logger.Warn("Certificate Store already exists. Key management provider and certificate store should not be configured together. Please migrate to key management provider and delete certificate store.")
 	}
 
-	provider, err := cutils.SpecToKeyManagementProvider(keyManagementProvider.Spec.Parameters.Raw, keyManagementProvider.Spec.Type)
+	provider, err := cutils.SpecToKeyManagementProvider(keyManagementProvider.Spec.Parameters.Raw, keyManagementProvider.Spec.Type, resource)
 	if err != nil {
 		kmpErr := re.ErrorCodeKeyManagementProviderFailure.WithError(err).WithDetail("Failed to create key management provider from CR")
 

pkg/controllers/namespaceresource/keymanagementprovider_controller.go

@@ -79,7 +79,7 @@ func (r *KeyManagementProviderReconciler) ReconcileWithType(ctx context.Context,
 		logger.Warn("Certificate Store already exists. Key management provider and certificate store should not be configured together. Please migrate to key management provider and delete certificate store.")
 	}
 
-	provider, err := cutils.SpecToKeyManagementProvider(keyManagementProvider.Spec.Parameters.Raw, keyManagementProvider.Spec.Type)
+	provider, err := cutils.SpecToKeyManagementProvider(keyManagementProvider.Spec.Parameters.Raw, keyManagementProvider.Spec.Type, resource)
 	if err != nil {
 		kmpErr := re.ErrorCodeKeyManagementProviderFailure.WithError(err).WithDetail("Failed to create key management provider from CR")
 

pkg/controllers/utils/kmp.go

@@ -26,8 +26,8 @@ import (
 )
 
 // SpecToKeyManagementProvider creates KeyManagementProvider from  KeyManagementProviderSpec config
-func SpecToKeyManagementProvider(raw []byte, keyManagamentSystemName string) (kmp.KeyManagementProvider, error) {
-	kmProviderConfig, err := rawToKeyManagementProviderConfig(raw, keyManagamentSystemName)
+func SpecToKeyManagementProvider(raw []byte, keyManagamentSystemName, resource string) (kmp.KeyManagementProvider, error) {
+	kmProviderConfig, err := rawToKeyManagementProviderConfig(raw, keyManagamentSystemName, resource)
 	if err != nil {
 		return nil, err
 	}
@@ -42,7 +42,7 @@ func SpecToKeyManagementProvider(raw []byte, keyManagamentSystemName string) (km
 }
 
 // rawToKeyManagementProviderConfig converts raw json to KeyManagementProviderConfig
-func rawToKeyManagementProviderConfig(raw []byte, keyManagamentSystemName string) (config.KeyManagementProviderConfig, error) {
+func rawToKeyManagementProviderConfig(raw []byte, keyManagamentSystemName, resource string) (config.KeyManagementProviderConfig, error) {
 	pluginConfig := config.KeyManagementProviderConfig{}
 
 	if string(raw) == "" {
@@ -53,6 +53,7 @@ func rawToKeyManagementProviderConfig(raw []byte, keyManagamentSystemName string
 	}
 
 	pluginConfig[types.Type] = keyManagamentSystemName
+	pluginConfig[types.Resource] = resource
 
 	return pluginConfig, nil
 }

pkg/controllers/utils/kmp_test.go

@@ -26,6 +26,7 @@ func TestSpecToKeyManagementProviderProvider(t *testing.T) {
 		name      string
 		raw       []byte
 		kmpType   string
+		resource  string
 		expectErr bool
 	}{
 		{
@@ -36,19 +37,21 @@ func TestSpecToKeyManagementProviderProvider(t *testing.T) {
 			name:      "missing inline provider required fields",
 			raw:       []byte("{\"type\": \"inline\"}"),
 			kmpType:   "inline",
+			resource:  "test",
 			expectErr: true,
 		},
 		{
 			name:      "valid spec",
 			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"}`),
 			kmpType:   "inline",
+			resource:  "test",
 			expectErr: false,
 		},
 	}
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			_, err := SpecToKeyManagementProvider(tc.raw, tc.kmpType)
+			_, err := SpecToKeyManagementProvider(tc.raw, tc.kmpType, tc.resource)
 			if tc.expectErr != (err != nil) {
 				t.Fatalf("Expected error to be %t, got %t", tc.expectErr, err != nil)
 			}
@@ -60,6 +63,7 @@ func TestSpecToKeyManagementProviderProvider(t *testing.T) {
 func TestRawToKeyManagementProviderConfig(t *testing.T) {
 	testCases := []struct {
 		name         string
+		resource     string
 		raw          []byte
 		expectErr    bool
 		expectConfig config.KeyManagementProviderConfig
@@ -72,23 +76,26 @@ func TestRawToKeyManagementProviderConfig(t *testing.T) {
 		},
 		{
 			name:         "unmarshal failure",
+			resource:     "test",
 			raw:          []byte("invalid"),
 			expectErr:    true,
 			expectConfig: config.KeyManagementProviderConfig{},
 		},
 		{
 			name:      "valid Raw",
+			resource:  "test",
 			raw:       []byte("{\"type\": \"inline\"}"),
 			expectErr: false,
 			expectConfig: config.KeyManagementProviderConfig{
-				"type": "inline",
+				"type":     "inline",
+				"resource": "test",
 			},
 		},
 	}
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			config, err := rawToKeyManagementProviderConfig(tc.raw, "inline")
+			config, err := rawToKeyManagementProviderConfig(tc.raw, "inline", "test")
 
 			if tc.expectErr != (err != nil) {
 				t.Fatalf("Expected error to be %t, got %t", tc.expectErr, err != nil)

pkg/keymanagementprovider/azurekeyvault/provider.go

@@ -25,6 +25,7 @@ import (
 	"encoding/json"
 	"encoding/pem"
 	"fmt"
+	"strconv"
 	"strings"
 	"time"
 
@@ -59,6 +60,7 @@ type AKVKeyManagementProviderConfig struct {
 	TenantID     string                `json:"tenantID"`
 	ClientID     string                `json:"clientID"`
 	CloudName    string                `json:"cloudName,omitempty"`
+	Resource     string                `json:"resource,omitempty"`
 	Certificates []types.KeyVaultValue `json:"certificates,omitempty"`
 	Keys         []types.KeyVaultValue `json:"keys,omitempty"`
 }
@@ -69,13 +71,33 @@ type akvKMProvider struct {
 	tenantID     string
 	clientID     string
 	cloudName    string
+	resource     string
 	certificates []types.KeyVaultValue
 	keys         []types.KeyVaultValue
 	cloudEnv     *azure.Environment
-	kvClient     *kv.BaseClient
+	kvClient     kvClient
 }
 type akvKMProviderFactory struct{}
 
+type kvClient interface {
+	GetCertificate(ctx context.Context, vaultBaseURL string, certificateName string, certificateVersion string) (kv.CertificateBundle, error)
+	GetKey(ctx context.Context, vaultBaseURL string, keyName string, keyVersion string) (kv.KeyBundle, error)
+	GetSecret(ctx context.Context, vaultBaseURL string, secretName string, secretVersion string) (kv.SecretBundle, error)
+}
+type kvClientImpl struct {
+	kv.BaseClient
+}
+
+func (c *kvClientImpl) GetCertificate(ctx context.Context, vaultBaseURL string, certificateName string, certificateVersion string) (kv.CertificateBundle, error) {
+	return c.BaseClient.GetCertificate(ctx, vaultBaseURL, certificateName, certificateVersion)
+}
+func (c *kvClientImpl) GetKey(ctx context.Context, vaultBaseURL string, keyName string, keyVersion string) (kv.KeyBundle, error) {
+	return c.BaseClient.GetKey(ctx, vaultBaseURL, keyName, keyVersion)
+}
+func (c *kvClientImpl) GetSecret(ctx context.Context, vaultBaseURL string, secretName string, secretVersion string) (kv.SecretBundle, error) {
+	return c.BaseClient.GetSecret(ctx, vaultBaseURL, secretName, secretVersion)
+}
+
 // initKVClient is a function to initialize the keyvault client
 // used for mocking purposes
 var initKVClient = initializeKvClient
@@ -116,6 +138,7 @@ func (f *akvKMProviderFactory) Create(_ string, keyManagementProviderConfig conf
 		certificates: conf.Certificates,
 		keys:         conf.Keys,
 		cloudEnv:     azureCloudEnv,
+		resource:     conf.Resource,
 	}
 	if err := provider.validate(); err != nil {
 		return nil, err
@@ -127,7 +150,7 @@ func (f *akvKMProviderFactory) Create(_ string, keyManagementProviderConfig conf
 	if err != nil {
 		return nil, re.ErrorCodePluginInitFailure.NewError(re.KeyManagementProvider, ProviderName, re.AKVLink, err, "failed to create keyvault client", re.HideStackTrace)
 	}
-	provider.kvClient = kvClient
+	provider.kvClient = &kvClientImpl{*kvClient}
 
 	return provider, nil
 }
@@ -141,46 +164,46 @@ func (s *akvKMProvider) GetCertificates(ctx context.Context) (map[keymanagementp
 		logger.GetLogger(ctx, logOpt).Debugf("fetching secret from key vault, certName %v,  keyvault %v", keyVaultCert.Name, s.vaultURI)
 
 		// fetch the object from Key Vault
+		startTime := time.Now()
 		certBundle, err := s.kvClient.GetCertificate(ctx, s.vaultURI, keyVaultCert.Name, keyVaultCert.Version)
 		if err != nil {
 			return nil, nil, fmt.Errorf("failed to get certificate objectName:%s, objectVersion:%s, error: %w", keyVaultCert.Name, keyVaultCert.Version, err)
 		}
 
-		//TODO: can I replace else with a continue?
-		if !*certBundle.Attributes.Enabled {
-			fmt.Printf("debug: certificate %s version %s is disabled.", keyVaultCert.Name, keyVaultCert.Version)
+		isEnabled := *certBundle.Attributes.Enabled
+		// if version is set as "" in the config, use the version from the cert bundle
+		keyVaultCert.Version = getObjectVersion(*certBundle.Kid)
+
+		if !isEnabled {
+			logger.GetLogger(ctx, logOpt).Debugf("debug: certificate %s version %s is disabled.", keyVaultCert.Name, keyVaultCert.Version)
 
-			isEnabled := "false"
+			isEnabled := false
 			startTime := time.Now()
 			lastRefreshed := startTime.Format(time.RFC3339)
 
-			metrics.ReportAKVCertificateDuration(ctx, time.Since(startTime).Milliseconds(), keyVaultCert.Name)
-			certProperty := getStatusProperty(keyVaultCert.Name, keyVaultCert.Version, isEnabled, lastRefreshed)
+			certProperty := getStatusProperty(keyVaultCert.Name, keyVaultCert.Version, strconv.FormatBool(isEnabled), lastRefreshed)
 			certsStatus = append(certsStatus, certProperty)
-			certMapKey := keymanagementprovider.KMPMapKey{Name: keyVaultCert.Name, Version: keyVaultCert.Version, Enabled: isEnabled}
-			certsMap[certMapKey] = []*x509.Certificate{} // empty cert chain
-		} else {
-			// GetSecret is required so we can fetch the entire cert chain. See issue https://github.com/ratify-project/ratify/issues/695 for details
-			isEnabled := "true"
-			startTime := time.Now()
-			secretBundle, err := s.kvClient.GetSecret(ctx, s.vaultURI, keyVaultCert.Name, keyVaultCert.Version)
-			if err != nil {
-				return nil, nil, fmt.Errorf("failed to get secret objectName:%s, objectVersion:%s, error: %w", keyVaultCert.Name, keyVaultCert.Version, err)
-			}
+			mapKey := keymanagementprovider.KMPMapKey{Name: keyVaultCert.Name, Version: keyVaultCert.Version, Enabled: isEnabled}
+			keymanagementprovider.DeleteCertificateFromMap(s.resource, mapKey)
+			continue
+		}
 
-			certResult, certProperty, err := getCertsFromSecretBundle(ctx, secretBundle, keyVaultCert.Name, isEnabled)
-			if err != nil {
-				return nil, nil, fmt.Errorf("failed to get certificates from secret bundle:%w", err)
-			}
+		// GetSecret is required so we can fetch the entire cert chain. See issue https://github.com/ratify-project/ratify/issues/695 for details
+		secretBundle, err := s.kvClient.GetSecret(ctx, s.vaultURI, keyVaultCert.Name, keyVaultCert.Version)
+		if err != nil {
+			return nil, nil, fmt.Errorf("failed to get secret objectName:%s, objectVersion:%s, error: %w", keyVaultCert.Name, keyVaultCert.Version, err)
+		}
 
-			metrics.ReportAKVCertificateDuration(ctx, time.Since(startTime).Milliseconds(), keyVaultCert.Name)
-			certsStatus = append(certsStatus, certProperty...)
-			certMapKey := keymanagementprovider.KMPMapKey{Name: keyVaultCert.Name, Version: keyVaultCert.Version, Enabled: isEnabled}
-			certsMap[certMapKey] = certResult
+		certResult, certProperty, err := getCertsFromSecretBundle(ctx, secretBundle, keyVaultCert.Name, strconv.FormatBool(isEnabled))
+		if err != nil {
+			return nil, nil, fmt.Errorf("failed to get certificates from secret bundle:%w", err)
 		}
 
+		metrics.ReportAKVCertificateDuration(ctx, time.Since(startTime).Milliseconds(), keyVaultCert.Name)
+		certsStatus = append(certsStatus, certProperty...)
+		certMapKey := keymanagementprovider.KMPMapKey{Name: keyVaultCert.Name, Version: keyVaultCert.Version, Enabled: isEnabled}
+		certsMap[certMapKey] = certResult
 	}
-
 	return certsMap, getStatusMap(certsStatus, types.CertificatesStatus), nil
 }
 
@@ -199,26 +222,27 @@ func (s *akvKMProvider) GetKeys(ctx context.Context) (map[keymanagementprovider.
 			return nil, nil, fmt.Errorf("failed to get key objectName:%s, objectVersion:%s, error: %w", keyVaultKey.Name, keyVaultKey.Version, err)
 		}
 
-		if keyBundle.Attributes != nil && keyBundle.Attributes.Enabled != nil && !*keyBundle.Attributes.Enabled {
-			isEnabled := "false"
+		isEnabled := *keyBundle.Attributes.Enabled
+		// if version is set as "" in the config, use the version from the key bundle
+		keyVaultKey.Version = getObjectVersion(*keyBundle.Key.Kid)
+
+		if !isEnabled {
 			startTime := time.Now()
 			lastRefreshed := startTime.Format(time.RFC3339)
-
-			keysMap[keymanagementprovider.KMPMapKey{Name: keyVaultKey.Name, Version: keyVaultKey.Version}] = nil
-			metrics.ReportAKVCertificateDuration(ctx, time.Since(startTime).Milliseconds(), keyVaultKey.Name)
-			properties := getStatusProperty(keyVaultKey.Name, keyVaultKey.Version, isEnabled, lastRefreshed)
+			properties := getStatusProperty(keyVaultKey.Name, keyVaultKey.Version, strconv.FormatBool(isEnabled), lastRefreshed)
 			keysStatus = append(keysStatus, properties)
-
+			mapKey := keymanagementprovider.KMPMapKey{Name: keyVaultKey.Name, Version: keyVaultKey.Version, Enabled: isEnabled}
+			keymanagementprovider.DeleteKeyFromMap(s.resource, mapKey)
 			continue
 		}
 
 		publicKey, err := getKeyFromKeyBundle(keyBundle)
 		if err != nil {
 			return nil, nil, fmt.Errorf("failed to get key from key bundle:%w", err)
 		}
-		keysMap[keymanagementprovider.KMPMapKey{Name: keyVaultKey.Name, Version: keyVaultKey.Version}] = publicKey
+		keysMap[keymanagementprovider.KMPMapKey{Name: keyVaultKey.Name, Version: keyVaultKey.Version, Enabled: isEnabled}] = publicKey
 		metrics.ReportAKVCertificateDuration(ctx, time.Since(startTime).Milliseconds(), keyVaultKey.Name)
-		properties := getStatusProperty(keyVaultKey.Name, keyVaultKey.Version, "true", time.Now().Format(time.RFC3339))
+		properties := getStatusProperty(keyVaultKey.Name, keyVaultKey.Version, strconv.FormatBool(isEnabled), time.Now().Format(time.RFC3339))
 		keysStatus = append(keysStatus, properties)
 	}
 
@@ -262,7 +286,7 @@ func initializeKvClient(ctx context.Context, keyVaultEndpoint, tenantID, clientI
 	kvClient := kv.New()
 	kvEndpoint := strings.TrimSuffix(keyVaultEndpoint, "/")
 
-	err := kvClient.AddToUserAgent(userAgent)
+	err := kvClient.Client.AddToUserAgent(userAgent)
 	if err != nil {
 		return nil, re.ErrorCodeConfigInvalid.WithDetail("Failed to add user agent to keyvault client.").WithRemediation(re.AKVLink).WithError(err)
 	}