Skip to content

Commit

Permalink
Merge pull request #2 from duffney/issue-1751/rm-disabled-certs
Browse files Browse the repository at this point in the history
fix: remove disabled certs from cache
  • Loading branch information
duffney authored Oct 30, 2024
2 parents 44141c6 + f194119 commit 12452a6
Show file tree
Hide file tree
Showing 7 changed files with 42 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Expand Down
7 changes: 4 additions & 3 deletions pkg/controllers/utils/kmp.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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) == "" {
Expand All @@ -53,6 +53,7 @@ func rawToKeyManagementProviderConfig(raw []byte, keyManagamentSystemName string
}

pluginConfig[types.Type] = keyManagamentSystemName
pluginConfig[types.Resource] = resource

return pluginConfig, nil
}
13 changes: 10 additions & 3 deletions pkg/controllers/utils/kmp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ func TestSpecToKeyManagementProviderProvider(t *testing.T) {
name string
raw []byte
kmpType string
resource string
expectErr bool
}{
{
Expand All @@ -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)
}
Expand All @@ -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
Expand All @@ -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)
Expand Down
12 changes: 7 additions & 5 deletions pkg/keymanagementprovider/azurekeyvault/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,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"`
}
Expand All @@ -70,6 +71,7 @@ type akvKMProvider struct {
tenantID string
clientID string
cloudName string
resource string
certificates []types.KeyVaultValue
keys []types.KeyVaultValue
cloudEnv *azure.Environment
Expand Down Expand Up @@ -136,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
Expand Down Expand Up @@ -180,8 +183,8 @@ func (s *akvKMProvider) GetCertificates(ctx context.Context) (map[keymanagementp

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
mapKey := keymanagementprovider.KMPMapKey{Name: keyVaultCert.Name, Version: keyVaultCert.Version, Enabled: isEnabled}
keymanagementprovider.DeleteCertificateFromMap(s.resource, mapKey)
continue
}

Expand Down Expand Up @@ -226,11 +229,10 @@ func (s *akvKMProvider) GetKeys(ctx context.Context) (map[keymanagementprovider.
if !isEnabled {
startTime := time.Now()
lastRefreshed := startTime.Format(time.RFC3339)

keysMap[keymanagementprovider.KMPMapKey{Name: keyVaultKey.Name, Version: keyVaultKey.Version, Enabled: isEnabled}] = nil
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
}

Expand Down
18 changes: 18 additions & 0 deletions pkg/keymanagementprovider/keymanagementprovider.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,24 @@ func DeleteResourceFromMap(resource string) {
keyErrMap.Delete(resource)
}

// DeleteKeyFromMap deletes the keys from the map
func DeleteCertificateFromMap(resource string, certKey KMPMapKey) {
if certs, ok := certificatesMap.Load(resource); ok {
if certMap, ok := certs.(map[KMPMapKey][]*x509.Certificate); ok {
delete(certMap, certKey)
}
}
}

// DeleteKeyFromMap deletes the keys from the map
func DeleteKeyFromMap(resource string, key KMPMapKey) {
if keys, ok := keyMap.Load(resource); ok {
if keyMap, ok := keys.(map[KMPMapKey]PublicKey); ok {
delete(keyMap, key)
}
}
}

// FlattenKMPMap flattens the map of certificates fetched for a single key management provider resource and returns a single array
func FlattenKMPMap(certMap map[KMPMapKey][]*x509.Certificate) []*x509.Certificate {
var items []*x509.Certificate
Expand Down
1 change: 1 addition & 0 deletions pkg/keymanagementprovider/types/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,6 @@ const (
SpecVersion string = "0.1.0"
Version string = "version"
Type string = "type"
Resource string = "resource"
Source string = "source"
)

0 comments on commit 12452a6

Please sign in to comment.