fix(policy)!: GetPC does not respect manager field.

otdfctl/cmd/policy/keyManagementProvider.go

@@ -47,8 +47,9 @@ func getProviderConfig(cmd *cobra.Command, args []string) {
 
 	id := c.Flags.GetOptionalID("id")
 	name := c.Flags.GetOptionalString("name")
+	manager := c.Flags.GetOptionalString("manager")
 
-	pc, err := h.GetProviderConfig(c.Context(), id, name)
+	pc, err := h.GetProviderConfig(c.Context(), id, name, manager)
 	if err != nil {
 		cli.ExitWithError("Failed to get provider config", err)
 	}
@@ -156,7 +157,7 @@ func deleteProviderConfig(cmd *cobra.Command, args []string) {
 	force := c.Flags.GetOptionalBool("force")
 
 	// Get provider config.
-	pc, err := h.GetProviderConfig(c.Context(), id, "")
+	pc, err := h.GetProviderConfig(c.Context(), id, "", "")
 	if err != nil {
 		cli.ExitWithError("Failed to get provider config", err)
 	}
@@ -218,8 +219,15 @@ func initKeyManagementProviderCommands() {
 		getDoc.GetDocFlag("name").Default,
 		getDoc.GetDocFlag("name").Description,
 	)
+	getDoc.Flags().StringP(
+		getDoc.GetDocFlag("manager").Name,
+		getDoc.GetDocFlag("manager").Shorthand,
+		getDoc.GetDocFlag("manager").Default,
+		getDoc.GetDocFlag("manager").Description,
+	)
 	getDoc.MarkFlagsOneRequired("id", "name")
 	getDoc.MarkFlagsMutuallyExclusive("id", "name")
+	getDoc.MarkFlagsRequiredTogether("name", "manager")
 
 	// Update Provider Config
 	updateDoc := man.Docs.GetCommand("policy/key-management/provider/update",

otdfctl/docs/man/policy/key-management/provider/get.md

@@ -11,12 +11,21 @@ command:
     - name: name
       shorthand: n
       description: Name of the provider config to retrieve
+    - name: manager
+      shorthand: m
+      description: Registered key manager implementation name tied to the provider config
 ---
 
-Retrieves a provider config by its ID or name.
+Retrieves a provider config by its ID or by name and manager.
+
+The `manager` value comes from the key manager implementation name, such as [`BasicManagerName`](https://github.com/opentdf/platform/blob/54d5c78bdf7a868e00ee83308a4593c86d1d4095/service/internal/security/basic_manager.go#L22). The `name` is the provider config name from `provider create` or `provider list`.
 
 ## Examples
 
 ```shell
 otdfctl keymanagement provider get --id '04ba179c-2f77-4e0d-90c5-fe4d1c9aa3f7'
 ```
+
+```shell
+otdfctl keymanagement provider get --name 'aws-dev' --manager 'opentdf.io/aws'
+```

otdfctl/e2e/provider-config.bats

@@ -8,6 +8,7 @@ setup_file() {
   export DEBUG_LEVEL="--log-level debug"
   export VALID_CONFIG='{"cached":"key"}'
   export BASE64_CONFIG="eyJjYWNoZWQiOiAia2V5In0="
+  export TEST_MANAGER="opentdf.io/basic"
 }
 
 setup() {
@@ -32,35 +33,36 @@ delete_pc_by_id() {
 # Create Provider Configuration
 #########
 @test "fail to create provider configuration without config" {
-    run_otdfctl_key_pc create --name test-value
+    run_otdfctl_key_pc create --name test-value --manager "$TEST_MANAGER"
     assert_failure
     assert_output --partial "Flag '--config' is required"
 }
 
 @test "fail to create provider configuration without name" {
-    run_otdfctl_key_pc create --config '{}'
+    run_otdfctl_key_pc create --manager "$TEST_MANAGER" --config '{}'
     assert_failure
     assert_output --partial "Flag '--name' is required"
 }
 
 @test "fail to create provider configuration with invalid config" {
-    run_otdfctl_key_pc create --name test-config --config test-value
+    run_otdfctl_key_pc create --name test-config --manager "$TEST_MANAGER" --config test-value
     assert_failure
     assert_output --partial "invalid_argument"
 }
 
 @test "create provider configuration" {
     CONFIG_NAME="test-config"
-    run_otdfctl_key_pc create --name "$CONFIG_NAME" --config '"$VALID_CONFIG"' --json
+    run_otdfctl_key_pc create --name "$CONFIG_NAME" --manager "$TEST_MANAGER" --config '"$VALID_CONFIG"' --json
     assert_success
     assert_equal "$(echo "$output" | jq -r .name)" "$CONFIG_NAME"
+    assert_equal "$(echo "$output" | jq -r .manager)" "$TEST_MANAGER"
     assert_equal "$(echo "$output" | jq -r .config_json)" "$BASE64_CONFIG"
     delete_pc_by_id "$(echo "$output" | jq -r .id)"
 }
 
 @test "get provider configuration by id" {
     CONFIG_NAME="test-config-2"
-    run_otdfctl_key_pc create --name "$CONFIG_NAME" --config '"$VALID_CONFIG"' --json
+    run_otdfctl_key_pc create --name "$CONFIG_NAME" --manager "$TEST_MANAGER" --config '"$VALID_CONFIG"' --json
     assert_success
     ID=$(echo "$output" | jq -r '.id')
     run_otdfctl_key_pc get --id "$ID" --json
@@ -71,31 +73,38 @@ delete_pc_by_id() {
  }
 
 
-@test "get provider configuration by name" {
+@test "get provider configuration by name and manager" {
     CONFIG_NAME="test-config-3"
-    run_otdfctl_key_pc create --name "$CONFIG_NAME" --config '"$VALID_CONFIG"' --json
+    run_otdfctl_key_pc create --name "$CONFIG_NAME" --manager "$TEST_MANAGER" --config '"$VALID_CONFIG"' --json
     assert_success
     NAME=$(echo "$output" | jq -r '.name')
-    run_otdfctl_key_pc get --name "$NAME" --json
+    run_otdfctl_key_pc get --name "$NAME" --manager "$TEST_MANAGER" --json
     assert_success
     assert_equal "$(echo "$output" | jq -r .name)" "$CONFIG_NAME"
+    assert_equal "$(echo "$output" | jq -r .manager)" "$TEST_MANAGER"
     assert_equal "$(echo "$output" | jq -r .config_json)" "$BASE64_CONFIG"
     delete_pc_by_id "$(echo "$output" | jq -r .id)"
 }
 
+@test "fail to get provider configuration - no manager with name" {
+    run_otdfctl_key_pc get --name test-config
+    assert_failure
+    assert_output --partial "missing [manager]"
+}
+
 @test "fail to get provider configuration - no required flags" {
      run_otdfctl_key_pc get
      assert_failure
 }
 
 @test "fail to get provider configuration with non-existent name" {
-    run_otdfctl_key_pc get --name non-existent-config
+    run_otdfctl_key_pc get --name non-existent-config --manager "$TEST_MANAGER"
     assert_failure
     assert_output --partial "Failed to get provider config: not_found"
 }
 @test "list provider configurations" {
     NAME="tst-config-4"
-    run_otdfctl_key_pc create --name "$NAME" --config '"$VALID_CONFIG"' --manager "fake-manager" --json
+    run_otdfctl_key_pc create --name "$NAME" --config '"$VALID_CONFIG"' --manager "$TEST_MANAGER" --json
     assert_success
     ID=$(echo "$output" | jq -r '.id')
     run_otdfctl_key_pc list --json
@@ -113,7 +122,7 @@ delete_pc_by_id() {
     UPDATED_NAME="test-config-5-updated"
     UPDATED_CONFIG='{"cached": "key-updated"}'
     BASE64_UPDATED_CONFIG='eyJjYWNoZWQiOiAia2V5LXVwZGF0ZWQifQ=='
-    run_otdfctl_key_pc create --name "$NAME" --config '"$VALID_CONFIG"' --json
+    run_otdfctl_key_pc create --name "$NAME" --manager "$TEST_MANAGER" --config '"$VALID_CONFIG"' --json
     assert_success
     ID=$(echo "$output" | jq -r '.id')
     run_otdfctl_key_pc update --id "$ID" --name "$UPDATED_NAME" --config "'$UPDATED_CONFIG'" --json
@@ -132,7 +141,7 @@ delete_pc_by_id() {
 
 @test "fail to update provider configuration - no optional flags" {
     NAME="test-config-6"
-    run_otdfctl_key_pc create --name "$NAME" --config '"$VALID_CONFIG"' --json
+    run_otdfctl_key_pc create --name "$NAME" --manager "$TEST_MANAGER" --config '"$VALID_CONFIG"' --json
     ID=$(echo "$output" | jq -r '.id')
     run_otdfctl_key_pc update --id "$ID"
     assert_failure
@@ -142,7 +151,7 @@ delete_pc_by_id() {
 
 @test "fail to update provider configuration - invalid config format" {
     NAME="test-config-7"
-    run_otdfctl_key_pc create --name "$NAME" --config '"$VALID_CONFIG"' --json
+    run_otdfctl_key_pc create --name "$NAME" --manager "$TEST_MANAGER" --config '"$VALID_CONFIG"' --json
     assert_success
     ID=$(echo "$output" | jq -r '.id')
     run_otdfctl_key_pc update --id "$ID" --config "{invalid: json}"
@@ -153,7 +162,7 @@ delete_pc_by_id() {
 
 @test "delete provider configuration -- success" {
   NAME="test-config-8"  
-  run_otdfctl_key_pc create --name "$NAME" --config '"$VALID_CONFIG"' --json
+  run_otdfctl_key_pc create --name "$NAME" --manager "$TEST_MANAGER" --config '"$VALID_CONFIG"' --json
   ID=$(echo "$output" | jq -r '.id')
   run_otdfctl_key_pc delete --id "$ID" --force
   assert_success
@@ -167,7 +176,7 @@ delete_pc_by_id() {
 
 @test "delete provider configuration fail -- no force" {
   NAME="test-config-9"
-  run_otdfctl_key_pc create --name "$NAME" --config '"$VALID_CONFIG"' --json
+  run_otdfctl_key_pc create --name "$NAME" --manager "$TEST_MANAGER" --config '"$VALID_CONFIG"' --json
   ID=$(echo "$output" | jq -r '.id')
   run_otdfctl_key_pc delete --id "$ID"
   assert_failure
@@ -177,5 +186,5 @@ delete_pc_by_id() {
 
 teardown_file() {
   # clear out all test env vars
-  unset HOST WITH_CREDS DEBUG_LEVEL VALID_CONFIG BASE64_CONFIG
-}
+  unset HOST WITH_CREDS DEBUG_LEVEL VALID_CONFIG BASE64_CONFIG TEST_MANAGER
+}

otdfctl/pkg/handlers/provider-config.go

@@ -29,7 +29,7 @@ func (h Handler) CreateProviderConfig(
 	return resp.GetProviderConfig(), nil
 }
 
-func (h Handler) GetProviderConfig(ctx context.Context, id, name string) (*policy.KeyProviderConfig, error) {
+func (h Handler) GetProviderConfig(ctx context.Context, id, name, manager string) (*policy.KeyProviderConfig, error) {
 	req := keymanagement.GetProviderConfigRequest{}
 	if id != "" {
 		req.Identifier = &keymanagement.GetProviderConfigRequest_Id{
@@ -39,6 +39,7 @@ func (h Handler) GetProviderConfig(ctx context.Context, id, name string) (*polic
 		req.Identifier = &keymanagement.GetProviderConfigRequest_Name{
 			Name: name,
 		}
+		req.Manager = manager
 	}
 
 	resp, err := h.sdk.KeyManagement.GetProviderConfig(ctx, &req)

service/integration/keymanagement_test.go

@@ -115,8 +115,10 @@ func (s *KeyManagementSuite) Test_CreateProviderConfig_CapitalizedName_Succeeds(
 	pc := s.createTestProviderConfig(providerName, validProviderConfig, nil)
 	pcIDs = append(pcIDs, pc.GetId())
 
-	pcGet, err := s.db.PolicyClient.GetProviderConfig(s.ctx, &keymanagement.GetProviderConfigRequest_Name{
-		Name: s.testProvider,
+	pcGet, err := s.db.PolicyClient.GetProviderConfig(s.ctx, &keymanagement.GetProviderConfigRequest{
+		Identifier: &keymanagement.GetProviderConfigRequest_Name{
+			Name: s.testProvider,
+		},
 	})
 	s.Require().NoError(err)
 	s.NotNil(pcGet)
@@ -133,8 +135,10 @@ func (s *KeyManagementSuite) Test_GetProviderConfig_WithId_Succeeds() {
 	pc := s.createTestProviderConfig(s.testProvider, validProviderConfig, nil)
 	pcIDs = append(pcIDs, pc.GetId())
 
-	pc, err := s.db.PolicyClient.GetProviderConfig(s.ctx, &keymanagement.GetProviderConfigRequest_Id{
-		Id: pc.GetId(),
+	pc, err := s.db.PolicyClient.GetProviderConfig(s.ctx, &keymanagement.GetProviderConfigRequest{
+		Identifier: &keymanagement.GetProviderConfigRequest_Id{
+			Id: pc.GetId(),
+		},
 	})
 	s.Require().NoError(err)
 	s.NotNil(pc)
@@ -149,15 +153,49 @@ func (s *KeyManagementSuite) Test_GetProviderConfig_WithName_Succeeds() {
 	pc := s.createTestProviderConfig(s.testProvider, validProviderConfig, nil)
 	pcIDs = append(pcIDs, pc.GetId())
 
-	pc, err := s.db.PolicyClient.GetProviderConfig(s.ctx, &keymanagement.GetProviderConfigRequest_Name{
-		Name: s.testProvider,
+	pc, err := s.db.PolicyClient.GetProviderConfig(s.ctx, &keymanagement.GetProviderConfigRequest{
+		Identifier: &keymanagement.GetProviderConfigRequest_Name{
+			Name: s.testProvider,
+		},
 	})
 	s.Require().NoError(err)
 	s.NotNil(pc)
 	s.Equal(s.testProvider, pc.GetName())
 	s.Equal(validProviderConfig, pc.GetConfigJson())
 }
 
+func (s *KeyManagementSuite) Test_GetProviderConfig_WithNameAndManager_Succeeds() {
+	pcIDs := make([]string, 0)
+	defer func() {
+		s.deleteTestProviderConfigs(pcIDs)
+	}()
+
+	pc1 := s.createTestProviderConfigWithManager(s.testProvider, validProviderConfig, nil)
+	pcIDs = append(pcIDs, pc1.GetId())
+
+	pc2, err := s.db.PolicyClient.CreateProviderConfig(s.ctx, &keymanagement.CreateProviderConfigRequest{
+		Name:       s.testProvider,
+		Manager:    "test-manager",
+		ConfigJson: validProviderConfig2,
+	})
+	s.Require().NoError(err)
+	s.NotNil(pc2)
+	pcIDs = append(pcIDs, pc2.GetId())
+
+	pc, err := s.db.PolicyClient.GetProviderConfig(s.ctx, &keymanagement.GetProviderConfigRequest{
+		Identifier: &keymanagement.GetProviderConfigRequest_Name{
+			Name: s.testProvider,
+		},
+		Manager: "test-manager",
+	})
+	s.Require().NoError(err)
+	s.NotNil(pc)
+	s.Equal(pc2.GetId(), pc.GetId())
+	s.Equal(s.testProvider, pc.GetName())
+	s.Equal("test-manager", pc.GetManager())
+	s.Equal(validProviderConfig2, pc.GetConfigJson())
+}
+
 func (s *KeyManagementSuite) Test_GetProviderConfig_MixedCaseName_Succeeds() {
 	pcIDs := make([]string, 0)
 	defer func() {
@@ -168,8 +206,10 @@ func (s *KeyManagementSuite) Test_GetProviderConfig_MixedCaseName_Succeeds() {
 	pc := s.createTestProviderConfig(mixedCaseName, validProviderConfig, nil)
 	pcIDs = append(pcIDs, pc.GetId())
 
-	pcGet, err := s.db.PolicyClient.GetProviderConfig(s.ctx, &keymanagement.GetProviderConfigRequest_Name{
-		Name: s.testProvider, // search with lowercase name
+	pcGet, err := s.db.PolicyClient.GetProviderConfig(s.ctx, &keymanagement.GetProviderConfigRequest{
+		Identifier: &keymanagement.GetProviderConfigRequest_Name{
+			Name: s.testProvider, // search with lowercase name
+		},
 	})
 	s.Require().NoError(err)
 	s.NotNil(pcGet)
@@ -178,7 +218,7 @@ func (s *KeyManagementSuite) Test_GetProviderConfig_MixedCaseName_Succeeds() {
 }
 
 func (s *KeyManagementSuite) Test_GetProviderConfig_InvalidIdentifier_Fails() {
-	pc, err := s.db.PolicyClient.GetProviderConfig(s.ctx, &map[string]string{})
+	pc, err := s.db.PolicyClient.GetProviderConfig(s.ctx, &keymanagement.GetProviderConfigRequest{})
 	s.Require().Error(err)
 	s.Nil(pc)
 }
@@ -653,16 +693,20 @@ func (s *KeyManagementSuite) Test_GetProviderConfig_IncludesManagerField() {
 	pcIDs = append(pcIDs, pc.GetId())
 
 	// Get by ID
-	retrievedByID, err := s.db.PolicyClient.GetProviderConfig(s.ctx, &keymanagement.GetProviderConfigRequest_Id{
-		Id: pc.GetId(),
+	retrievedByID, err := s.db.PolicyClient.GetProviderConfig(s.ctx, &keymanagement.GetProviderConfigRequest{
+		Identifier: &keymanagement.GetProviderConfigRequest_Id{
+			Id: pc.GetId(),
+		},
 	})
 	s.Require().NoError(err)
 	s.NotNil(retrievedByID)
 	s.Equal("opentdf.io/basic", retrievedByID.GetManager())
 
 	// Get by Name
-	retrievedByName, err := s.db.PolicyClient.GetProviderConfig(s.ctx, &keymanagement.GetProviderConfigRequest_Name{
-		Name: s.testProvider,
+	retrievedByName, err := s.db.PolicyClient.GetProviderConfig(s.ctx, &keymanagement.GetProviderConfigRequest{
+		Identifier: &keymanagement.GetProviderConfigRequest_Name{
+			Name: s.testProvider,
+		},
 	})
 	s.Require().NoError(err)
 	s.NotNil(retrievedByName)

service/policy/db/key_management.go

@@ -47,10 +47,10 @@ func (c PolicyDBClient) CreateProviderConfig(ctx context.Context, r *keymanageme
 	}, nil
 }
 
-func (c PolicyDBClient) GetProviderConfig(ctx context.Context, identifier any) (*policy.KeyProviderConfig, error) {
+func (c PolicyDBClient) GetProviderConfig(ctx context.Context, r *keymanagement.GetProviderConfigRequest) (*policy.KeyProviderConfig, error) {
 	var params getProviderConfigParams
 
-	switch i := identifier.(type) {
+	switch i := r.GetIdentifier().(type) {
 	case *keymanagement.GetProviderConfigRequest_Id:
 		id := pgtypeUUID(i.Id)
 		if !id.Valid {
@@ -62,7 +62,8 @@ func (c PolicyDBClient) GetProviderConfig(ctx context.Context, identifier any) (
 		if !name.Valid {
 			return nil, db.ErrSelectIdentifierInvalid
 		}
-		params = getProviderConfigParams{Name: name}
+		params.Name = name
+		params.Manager = pgtypeText(r.GetManager())
 	default:
 		// unexpected type
 		return nil, errors.Join(db.ErrUnknownSelectIdentifier, fmt.Errorf("type [%T] value [%v]", i, i))
@@ -144,8 +145,10 @@ func (c PolicyDBClient) UpdateProviderConfig(ctx context.Context, r *keymanageme
 
 	// if extend we need to merge the metadata
 	metadataJSON, _, err := db.MarshalUpdateMetadata(r.GetMetadata(), r.GetMetadataUpdateBehavior(), func() (*common.Metadata, error) {
-		a, err := c.GetProviderConfig(ctx, &keymanagement.GetProviderConfigRequest_Id{
-			Id: r.GetId(),
+		a, err := c.GetProviderConfig(ctx, &keymanagement.GetProviderConfigRequest{
+			Identifier: &keymanagement.GetProviderConfigRequest_Id{
+				Id: r.GetId(),
+			},
 		})
 		if err != nil {
 			return nil, err
@@ -173,8 +176,10 @@ func (c PolicyDBClient) UpdateProviderConfig(ctx context.Context, r *keymanageme
 		c.logger.Warn("updateProviderConfig updated more than one row", slog.Int64("count", count))
 	}
 
-	return c.GetProviderConfig(ctx, &keymanagement.GetProviderConfigRequest_Id{
-		Id: id,
+	return c.GetProviderConfig(ctx, &keymanagement.GetProviderConfigRequest{
+		Identifier: &keymanagement.GetProviderConfigRequest_Id{
+			Id: id,
+		},
 	})
 }