remove undocumented requirement for gcpkms key_identifier_values to be 36 characters

pkg/server/plugin/keymanager/gcpkms/fetcher.go

@@ -46,7 +46,7 @@ func (kf *keyFetcher) fetchKeyEntries(ctx context.Context) ([]*keyEntry, error)
 		if err != nil {
 			return nil, status.Errorf(codes.Internal, "failed to list SPIRE Server keys in Cloud KMS: %v", err)
 		}
-		spireKeyID, ok := getSPIREKeyIDFromCryptoKeyName(cryptoKey.Name)
+		spireKeyID, ok := getSPIREKeyIDFromCryptoKeyName(cryptoKey.Name, kf.serverID)
 		if !ok {
 			kf.log.Warn("Could not get SPIRE Key ID from CryptoKey", cryptoKeyNameTag, cryptoKey.Name)
 			continue
@@ -130,29 +130,27 @@ func (kf *keyFetcher) getKeyEntriesFromCryptoKey(ctx context.Context, cryptoKey
 
 // getSPIREKeyIDFromCryptoKeyName parses a CryptoKey resource name to get the
 // SPIRE Key ID. This Key ID is used in the Server KeyManager interface.
-func getSPIREKeyIDFromCryptoKeyName(cryptoKeyName string) (string, bool) {
+func getSPIREKeyIDFromCryptoKeyName(cryptoKeyName, serverID string) (string, bool) {
 	// cryptoKeyName is the resource name for the CryptoKey holding the SPIRE Key
-	// in the format: projects/*/locations/*/keyRings/*/cryptoKeys/spire-key-*-*.
+	// in the format: projects/*/locations/*/keyRings/*/cryptoKeys/spire-key-<serverID>-<spireKeyID>.
 	// Example: projects/project-name/locations/us-east1/keyRings/key-ring-name/cryptoKeys/spire-key-1f2e225a-91d8-4589-a4fe-f88b7bb04bac-x509-CA-A
+	//
+	// The server ID is not fixed-length (it is a UUID with key_identifier_file
+	// but an arbitrary value with key_identifier_value), so it must be stripped
+	// using the known prefix built by generateCryptoKeyID rather than a fixed
+	// offset.
 
-	// Get the last element of the path.
+	// Get the CryptoKeyID (last element of the path).
 	i := strings.LastIndex(cryptoKeyName, "/")
 	if i < 0 {
-		// All CryptoKeys are under a Key Ring; not a valid Crypto Key name.
 		return "", false
 	}
-
-	// The i index will indicate us where
-	// "spire-key-1f2e225a-91d8-4589-a4fe-f88b7bb04bac-x509-CA-A" starts.
-	// Now we have to get the position where the SPIRE Key ID starts.
-	// For that, we need to add the length of the CryptoKey name prefix that we
-	// are using, the UUID length, and the two "-" separators used in our format.
-	spireKeyIDIndex := i + len(cryptoKeyNamePrefix) + 39 // 39 is the UUID length plus two '-' separators
-	if spireKeyIDIndex >= len(cryptoKeyName) {
-		// The index is out of range.
+	cryptoKeyID := cryptoKeyName[i+1:]
+	prefix := fmt.Sprintf("%s-%s-", cryptoKeyNamePrefix, serverID)
+	spireKeyID, ok := strings.CutPrefix(cryptoKeyID, prefix)
+	if !ok || spireKeyID == "" {
 		return "", false
 	}
-	spireKeyID := cryptoKeyName[spireKeyIDIndex:]
 	return spireKeyID, true
 }
 

pkg/server/plugin/keymanager/gcpkms/gcpkms_test.go

@@ -430,7 +430,7 @@ func TestConfigure(t *testing.T) {
 			// Assert that the keys have been loaded
 			storedFakeCryptoKeys := ts.fakeKMSClient.store.fetchFakeCryptoKeys()
 			for _, expectedFakeCryptoKey := range storedFakeCryptoKeys {
-				spireKeyID, ok := getSPIREKeyIDFromCryptoKeyName(expectedFakeCryptoKey.Name)
+				spireKeyID, ok := getSPIREKeyIDFromCryptoKeyName(expectedFakeCryptoKey.Name, ts.plugin.pd.serverID)
 				require.True(t, ok)
 
 				entry, ok := ts.plugin.entries[spireKeyID]
@@ -443,6 +443,56 @@ func TestConfigure(t *testing.T) {
 	}
 }
 
+func TestGetSPIREKeyIDFromCryptoKeyName(t *testing.T) {
+	for _, tt := range []struct {
+		name          string
+		cryptoKeyName string
+		serverID      string
+		expectKeyID   string
+		expectOK      bool
+	}{
+		{
+			name:          "uuid server id (key_identifier_file)",
+			cryptoKeyName: path.Join(validKeyRing, "cryptoKeys", fmt.Sprintf("spire-key-%s-x509-CA-A", validServerID)),
+			serverID:      validServerID,
+			expectKeyID:   "x509-CA-A",
+			expectOK:      true,
+		},
+		{
+			name:          "short custom server id (key_identifier_value)",
+			cryptoKeyName: path.Join(validKeyRing, "cryptoKeys", "spire-key-dpt-gcp-38d0-x509-CA-A"),
+			serverID:      "dpt-gcp-38d0",
+			expectKeyID:   "x509-CA-A",
+			expectOK:      true,
+		},
+		{
+			name:          "server id containing dashes (key_identifier_value)",
+			cryptoKeyName: path.Join(validKeyRing, "cryptoKeys", "spire-key-my-server-JWT-B"),
+			serverID:      "my-server",
+			expectKeyID:   "JWT-B",
+			expectOK:      true,
+		},
+		{
+			name:          "prefix does not match server id",
+			cryptoKeyName: path.Join(validKeyRing, "cryptoKeys", "spire-key-other-server-x509-CA-A"),
+			serverID:      "dpt-gcp-38d0",
+			expectOK:      false,
+		},
+		{
+			name:          "no spire key id after server id",
+			cryptoKeyName: path.Join(validKeyRing, "cryptoKeys", "spire-key-dpt-gcp-38d0"),
+			serverID:      "dpt-gcp-38d0",
+			expectOK:      false,
+		},
+	} {
+		t.Run(tt.name, func(t *testing.T) {
+			spireKeyID, ok := getSPIREKeyIDFromCryptoKeyName(tt.cryptoKeyName, tt.serverID)
+			require.Equal(t, tt.expectOK, ok)
+			require.Equal(t, tt.expectKeyID, spireKeyID)
+		})
+	}
+}
+
 func TestDisposeStaleCryptoKeys(t *testing.T) {
 	configureRequest := configureRequestWithDefaults(t)
 	ts := setupTest(t)