From b25d404b308d488a41aa39c6116eada4c833e1f8 Mon Sep 17 00:00:00 2001 From: Brett Dudo Date: Wed, 27 May 2026 18:06:47 -0700 Subject: [PATCH 1/5] feat: expose Vault group ID in Group CRD status After creating or updating a Vault identity group, read back the group from Vault and persist its ID in status.id. This allows other resources (e.g., KRO ResourceGraphDefinitions) to reference the group ID for nesting groups via memberGroupIDs. Introduces a VaultStatusEnricher interface so other CRDs can opt into similar status enrichment without changing the generic reconciler flow. Closes #326 --- api/v1alpha1/group_types.go | 21 +++++++++++++++++++ api/v1alpha1/utils/vaultobject.go | 7 +++++++ .../vaultresourcereconciler.go | 8 +++++++ 3 files changed, 36 insertions(+) diff --git a/api/v1alpha1/group_types.go b/api/v1alpha1/group_types.go index e51e5e82..d03b2845 100644 --- a/api/v1alpha1/group_types.go +++ b/api/v1alpha1/group_types.go @@ -20,6 +20,7 @@ import ( "context" "reflect" + vault "github.com/hashicorp/vault/api" vaultutils "github.com/redhat-cop/vault-config-operator/api/v1alpha1/utils" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" @@ -80,6 +81,10 @@ type GroupConfig struct { // GroupStatus defines the observed state of Group type GroupStatus struct { + // ID is the Vault-assigned unique identifier for this identity group. + // +kubebuilder:validation:Optional + ID string `json:"id,omitempty"` + // +patchMergeKey=type // +patchStrategy=merge // +listType=map @@ -114,6 +119,7 @@ func init() { var _ vaultutils.VaultObject = &Group{} var _ vaultutils.ConditionsAware = &Group{} +var _ vaultutils.VaultStatusEnricher = &Group{} func (m *Group) GetConditions() []metav1.Condition { return m.Status.Conditions @@ -178,3 +184,18 @@ func (d *Group) IsEquivalentToDesiredState(payload map[string]interface{}) bool desiredState := d.Spec.toMap() return reflect.DeepEqual(desiredState, filterPayloadToDesiredKeys(desiredState, payload)) } + +// EnrichStatus reads the group back from Vault and persists the Vault-assigned ID in status. +func (d *Group) EnrichStatus(ctx context.Context) error { + vaultClient := ctx.Value("vaultClient").(*vault.Client) + secret, err := vaultClient.Logical().ReadWithContext(ctx, d.GetPath()) + if err != nil { + return err + } + if secret != nil && secret.Data != nil { + if id, ok := secret.Data["id"].(string); ok { + d.Status.ID = id + } + } + return nil +} diff --git a/api/v1alpha1/utils/vaultobject.go b/api/v1alpha1/utils/vaultobject.go index 23b3ec5d..f24ed8b6 100644 --- a/api/v1alpha1/utils/vaultobject.go +++ b/api/v1alpha1/utils/vaultobject.go @@ -39,6 +39,13 @@ type VaultObject interface { GetVaultConnection() *VaultConnection } +// VaultStatusEnricher is an optional interface that VaultObjects can implement +// to enrich their Kubernetes status with data read back from Vault after a +// successful create or update operation. +type VaultStatusEnricher interface { + EnrichStatus(ctx context.Context) error +} + type VaultEndpoint struct { vaultObject VaultObject } diff --git a/controllers/vaultresourcecontroller/vaultresourcereconciler.go b/controllers/vaultresourcecontroller/vaultresourcereconciler.go index 0e6d81f7..f7402f62 100644 --- a/controllers/vaultresourcecontroller/vaultresourcereconciler.go +++ b/controllers/vaultresourcecontroller/vaultresourcereconciler.go @@ -68,6 +68,14 @@ func (r *VaultResource) Reconcile(ctx context.Context, instance client.Object) ( return ManageOutcome(ctx, *r.reconcilerBase, instance, err) } + // Enrich status with Vault-side data if the object supports it + if enricher, ok := instance.(vaultutils.VaultStatusEnricher); ok { + if enrichErr := enricher.EnrichStatus(ctx); enrichErr != nil { + log.Error(enrichErr, "unable to enrich status from Vault", "instance", instance) + // Non-fatal: proceed with ManageOutcome so conditions are still updated + } + } + return ManageOutcome(ctx, *r.reconcilerBase, instance, err) } From 850030259a7dc32442511e7054ad2c4363f08ae3 Mon Sep 17 00:00:00 2001 From: Brett Dudo Date: Wed, 1 Jul 2026 10:14:22 -0700 Subject: [PATCH 2/5] =?UTF-8?q?chore:=20rebase=20onto=20main=20=E2=80=94?= =?UTF-8?q?=20map[string]any,=20VaultClientFromContext,=20new=20reconciler?= =?UTF-8?q?=20structure?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- api/v1alpha1/group_types.go | 11 ++- api/v1alpha1/utils/vaultobject.go | 18 ++--- .../vaultresourcereconciler.go | 73 ++++--------------- 3 files changed, 27 insertions(+), 75 deletions(-) diff --git a/api/v1alpha1/group_types.go b/api/v1alpha1/group_types.go index d03b2845..8ab5f2c7 100644 --- a/api/v1alpha1/group_types.go +++ b/api/v1alpha1/group_types.go @@ -20,7 +20,6 @@ import ( "context" "reflect" - vault "github.com/hashicorp/vault/api" vaultutils "github.com/redhat-cop/vault-config-operator/api/v1alpha1/utils" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" @@ -144,12 +143,12 @@ func (d *Group) GetPath() string { return vaultutils.CleansePath(string("/identity/group/name/" + d.Name)) } -func (d *Group) GetPayload() map[string]interface{} { +func (d *Group) GetPayload() map[string]any { return d.Spec.toMap() } -func (i *GroupSpec) toMap() map[string]interface{} { - payload := map[string]interface{}{} +func (i *GroupSpec) toMap() map[string]any { + payload := map[string]any{} payload["type"] = i.Type payload["metadata"] = i.Metadata payload["policies"] = i.Policies @@ -180,14 +179,14 @@ func (d *Group) GetKubeAuthConfiguration() *vaultutils.KubeAuthConfiguration { return &d.Spec.Authentication } -func (d *Group) IsEquivalentToDesiredState(payload map[string]interface{}) bool { +func (d *Group) IsEquivalentToDesiredState(payload map[string]any) bool { desiredState := d.Spec.toMap() return reflect.DeepEqual(desiredState, filterPayloadToDesiredKeys(desiredState, payload)) } // EnrichStatus reads the group back from Vault and persists the Vault-assigned ID in status. func (d *Group) EnrichStatus(ctx context.Context) error { - vaultClient := ctx.Value("vaultClient").(*vault.Client) + vaultClient := vaultutils.VaultClientFromContext(ctx) secret, err := vaultClient.Logical().ReadWithContext(ctx, d.GetPath()) if err != nil { return err diff --git a/api/v1alpha1/utils/vaultobject.go b/api/v1alpha1/utils/vaultobject.go index f24ed8b6..ee23523a 100644 --- a/api/v1alpha1/utils/vaultobject.go +++ b/api/v1alpha1/utils/vaultobject.go @@ -27,9 +27,9 @@ import ( type VaultObject interface { GetPath() string - GetPayload() map[string]interface{} + GetPayload() map[string]any // IsEquivalentToDesiredState returns wether the passed payload is equivalent to the payload that the current object would generate. When this is a engine object the tune payload will be compared - IsEquivalentToDesiredState(payload map[string]interface{}) bool + IsEquivalentToDesiredState(payload map[string]any) bool IsInitialized() bool IsValid() (bool, error) IsDeletable() bool @@ -60,7 +60,7 @@ func NewVaultEndpoint(obj client.Object) *VaultEndpoint { // This is similar to vaultClient.KVv2(mountPath string).DeleteMetadata(ctx context.Context, secretPath string) but works better with existing interface func (ve *VaultEndpoint) DeleteKVv2IfExists(context context.Context) error { log := log.FromContext(context) - vaultClient := context.Value("vaultClient").(*vault.Client) + vaultClient := VaultClientFromContext(context) // should match pathToDelete := fmt.Sprintf("%s/metadata/%s", kv.mountPath, secretPath) pathToDelete := strings.Replace(ve.vaultObject.GetPath(), "/data/", "/metadata/", 1) @@ -80,7 +80,7 @@ func (ve *VaultEndpoint) DeleteKVv2IfExists(context context.Context) error { func (ve *VaultEndpoint) DeleteIfExists(context context.Context) error { log := log.FromContext(context) - vaultClient := context.Value("vaultClient").(*vault.Client) + vaultClient := VaultClientFromContext(context) _, err := vaultClient.Logical().Delete(ve.vaultObject.GetPath()) if err != nil { if respErr, ok := err.(*vault.ResponseError); ok { @@ -128,11 +128,11 @@ func (ve *VaultEndpoint) CreateOrMergeKV(context context.Context, isKVv2 bool, p newPayload := ve.vaultObject.GetPayload() if isKVv2 { // For KVv2, data is nested under "data" key - existingData, ok := currentPayload["data"].(map[string]interface{}) + existingData, ok := currentPayload["data"].(map[string]any) if !ok { - existingData = make(map[string]interface{}) + existingData = make(map[string]any) } - newData, ok := newPayload["data"].(map[string]interface{}) + newData, ok := newPayload["data"].(map[string]any) if !ok { return write(context, ve.vaultObject.GetPath(), newPayload) } @@ -145,7 +145,7 @@ func (ve *VaultEndpoint) CreateOrMergeKV(context context.Context, isKVv2 bool, p } existingData[k] = v } - mergedPayload := map[string]interface{}{ + mergedPayload := map[string]any{ "data": existingData, } return write(context, ve.vaultObject.GetPath(), mergedPayload) @@ -183,7 +183,7 @@ func (ve *VaultEndpoint) CreateOrUpdate(context context.Context) error { type RabbitMQEngineConfigVaultObject interface { VaultObject GetLeasePath() string - GetLeasePayload() map[string]interface{} + GetLeasePayload() map[string]any CheckTTLValuesProvided() bool } diff --git a/controllers/vaultresourcecontroller/vaultresourcereconciler.go b/controllers/vaultresourcecontroller/vaultresourcereconciler.go index f7402f62..8b5615fb 100644 --- a/controllers/vaultresourcecontroller/vaultresourcereconciler.go +++ b/controllers/vaultresourcecontroller/vaultresourcereconciler.go @@ -20,12 +20,9 @@ import ( "context" vaultutils "github.com/redhat-cop/vault-config-operator/api/v1alpha1/utils" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/log" - "sigs.k8s.io/controller-runtime/pkg/reconcile" ) type VaultResource struct { @@ -41,63 +38,10 @@ func NewVaultResource(reconcilerBase *ReconcilerBase, obj client.Object) *VaultR } func (r *VaultResource) Reconcile(ctx context.Context, instance client.Object) (ctrl.Result, error) { - log := log.FromContext(ctx) - log.Info("starting reconcile cycle") - log.V(1).Info("reconcile", "instance", instance) - if !instance.GetDeletionTimestamp().IsZero() { - if !controllerutil.ContainsFinalizer(instance, vaultutils.GetFinalizer(instance)) { - return reconcile.Result{}, nil - } - err := r.manageCleanUpLogic(ctx, instance) - if err != nil { - log.Error(err, "unable to delete instance", "instance", instance) - return ManageOutcome(ctx, *r.reconcilerBase, instance, err) - } - controllerutil.RemoveFinalizer(instance, vaultutils.GetFinalizer(instance)) - err = r.reconcilerBase.GetClient().Update(ctx, instance) - if err != nil { - log.Error(err, "unable to update instance", "instance", instance) - return ManageOutcome(ctx, *r.reconcilerBase, instance, err) - } - return reconcile.Result{}, nil - } - - err := r.manageReconcileLogic(ctx, instance) - if err != nil { - log.Error(err, "unable to complete reconcile logic", "instance", instance) - return ManageOutcome(ctx, *r.reconcilerBase, instance, err) - } - - // Enrich status with Vault-side data if the object supports it - if enricher, ok := instance.(vaultutils.VaultStatusEnricher); ok { - if enrichErr := enricher.EnrichStatus(ctx); enrichErr != nil { - log.Error(enrichErr, "unable to enrich status from Vault", "instance", instance) - // Non-fatal: proceed with ManageOutcome so conditions are still updated - } - } - - return ManageOutcome(ctx, *r.reconcilerBase, instance, err) -} - -func (r *VaultResource) manageCleanUpLogic(context context.Context, instance client.Object) error { - log := log.FromContext(context) - if vaultObject, ok := instance.(vaultutils.VaultObject); ok { - if !vaultObject.IsDeletable() { - return nil - } - } - if conditionAware, ok := instance.(vaultutils.ConditionsAware); ok { - for _, condition := range conditionAware.GetConditions() { - if condition.Status == metav1.ConditionTrue && condition.Type == ReconcileSuccessful { - err := r.vaultEndpoint.DeleteIfExists(context) - if err != nil { - log.Error(err, "unable to delete vault resource", "instance", instance) - return err - } - } - } - } - return nil + return ReconcileWithFunctions(ctx, r.reconcilerBase, instance, + r.vaultEndpoint.DeleteIfExists, + r.manageReconcileLogic, + ) } func (r *VaultResource) manageReconcileLogic(context context.Context, instance client.Object) error { @@ -120,5 +64,14 @@ func (r *VaultResource) manageReconcileLogic(context context.Context, instance c log.Error(err, "unable to create/update vault resource", "instance", instance) return err } + + // Enrich status with Vault-side data if the object supports it + if enricher, ok := instance.(vaultutils.VaultStatusEnricher); ok { + if enrichErr := enricher.EnrichStatus(context); enrichErr != nil { + log.Error(enrichErr, "unable to enrich status from Vault", "instance", instance) + // Non-fatal: proceed so conditions are still updated + } + } + return nil } From 633d08a4a924ffea908a921fc1b957035ae988b8 Mon Sep 17 00:00:00 2001 From: Brett Dudo Date: Wed, 1 Jul 2026 10:25:01 -0700 Subject: [PATCH 3/5] test: add group_types_test.go covering EnrichStatus and core methods --- api/v1alpha1/group_types_test.go | 194 +++++++++++++++++++++++++++++++ 1 file changed, 194 insertions(+) create mode 100644 api/v1alpha1/group_types_test.go diff --git a/api/v1alpha1/group_types_test.go b/api/v1alpha1/group_types_test.go new file mode 100644 index 00000000..4fed7163 --- /dev/null +++ b/api/v1alpha1/group_types_test.go @@ -0,0 +1,194 @@ +package v1alpha1 + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + + vault "github.com/hashicorp/vault/api" + vaultutils "github.com/redhat-cop/vault-config-operator/api/v1alpha1/utils" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestGroupGetPath(t *testing.T) { + tests := []struct { + name string + group *Group + expected string + }{ + { + name: "uses metadata name when spec name is empty", + group: &Group{ + ObjectMeta: metav1.ObjectMeta{Name: "my-group"}, + }, + expected: "identity/group/name/my-group", + }, + { + name: "spec name takes precedence over metadata name", + group: &Group{ + ObjectMeta: metav1.ObjectMeta{Name: "my-group"}, + Spec: GroupSpec{Name: "override-name"}, + }, + expected: "identity/group/name/override-name", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := tt.group.GetPath(); got != tt.expected { + t.Errorf("GetPath() = %q, want %q", got, tt.expected) + } + }) + } +} + +func TestGroupGetPayloadInternalIncludesMemberFields(t *testing.T) { + group := &Group{ + Spec: GroupSpec{ + GroupConfig: GroupConfig{ + Type: "internal", + Policies: []string{"default"}, + MemberGroupIDs: []string{"group-1"}, + MemberEntityIDs: []string{"entity-1"}, + }, + }, + } + payload := group.GetPayload() + if payload["type"] != "internal" { + t.Errorf("expected type 'internal', got %v", payload["type"]) + } + if _, ok := payload["member_group_ids"]; !ok { + t.Error("expected member_group_ids to be present for internal group") + } + if _, ok := payload["member_entity_ids"]; !ok { + t.Error("expected member_entity_ids to be present for internal group") + } +} + +func TestGroupGetPayloadExternalOmitsMemberFields(t *testing.T) { + group := &Group{ + Spec: GroupSpec{ + GroupConfig: GroupConfig{ + Type: "external", + }, + }, + } + payload := group.GetPayload() + if _, ok := payload["member_group_ids"]; ok { + t.Error("expected member_group_ids to be absent for external group") + } + if _, ok := payload["member_entity_ids"]; ok { + t.Error("expected member_entity_ids to be absent for external group") + } +} + +func TestGroupIsEquivalentToDesiredState(t *testing.T) { + group := &Group{ + Spec: GroupSpec{ + GroupConfig: GroupConfig{ + Type: "internal", + Policies: []string{"default"}, + }, + }, + } + if !group.IsEquivalentToDesiredState(group.GetPayload()) { + t.Error("expected group to be equivalent to its own payload") + } +} + +func TestGroupIsEquivalentToDesiredStateMismatch(t *testing.T) { + group := &Group{ + Spec: GroupSpec{ + GroupConfig: GroupConfig{ + Type: "internal", + Policies: []string{"default"}, + }, + }, + } + payload := group.GetPayload() + payload["type"] = "external" + if group.IsEquivalentToDesiredState(payload) { + t.Error("expected group to not be equivalent to payload with different type") + } +} + +// newVaultTestClient creates a vault.Client pointed at the given httptest.Server. +func newVaultTestClient(t *testing.T, srv *httptest.Server) *vault.Client { + t.Helper() + cfg := vault.DefaultConfig() + cfg.Address = srv.URL + client, err := vault.NewClient(cfg) + if err != nil { + t.Fatalf("vault.NewClient: %v", err) + } + client.SetToken("test-token") + return client +} + +func TestGroupEnrichStatus(t *testing.T) { + const wantID = "abc-123-def-456" + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode(map[string]any{ + "data": map[string]any{ + "id": wantID, + "name": "my-group", + }, + }) + })) + defer srv.Close() + + group := &Group{ + ObjectMeta: metav1.ObjectMeta{Name: "my-group"}, + } + ctx := vaultutils.ContextWithVaultClient(context.Background(), newVaultTestClient(t, srv)) + + if err := group.EnrichStatus(ctx); err != nil { + t.Fatalf("EnrichStatus() unexpected error: %v", err) + } + if group.Status.ID != wantID { + t.Errorf("Status.ID = %q, want %q", group.Status.ID, wantID) + } +} + +func TestGroupEnrichStatusNoIDField(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode(map[string]any{ + "data": map[string]any{ + "name": "my-group", // no "id" key + }, + }) + })) + defer srv.Close() + + group := &Group{ + ObjectMeta: metav1.ObjectMeta{Name: "my-group"}, + } + ctx := vaultutils.ContextWithVaultClient(context.Background(), newVaultTestClient(t, srv)) + + if err := group.EnrichStatus(ctx); err != nil { + t.Fatalf("EnrichStatus() unexpected error: %v", err) + } + if group.Status.ID != "" { + t.Errorf("expected Status.ID to remain empty, got %q", group.Status.ID) + } +} + +func TestGroupEnrichStatusVaultError(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Error(w, `{"errors":["internal server error"]}`, http.StatusInternalServerError) + })) + defer srv.Close() + + group := &Group{ + ObjectMeta: metav1.ObjectMeta{Name: "my-group"}, + } + ctx := vaultutils.ContextWithVaultClient(context.Background(), newVaultTestClient(t, srv)) + + if err := group.EnrichStatus(ctx); err == nil { + t.Error("expected error from Vault 500 response, got nil") + } +} From f22c16b2dde48f55521db555566995c66ea1e302 Mon Sep 17 00:00:00 2001 From: Brett Dudo Date: Thu, 2 Jul 2026 12:57:41 -0700 Subject: [PATCH 4/5] test: remove duplicate group_types_test.go (tests belong in group_test.go) --- api/v1alpha1/group_types_test.go | 194 ------------------------------- 1 file changed, 194 deletions(-) delete mode 100644 api/v1alpha1/group_types_test.go diff --git a/api/v1alpha1/group_types_test.go b/api/v1alpha1/group_types_test.go deleted file mode 100644 index 4fed7163..00000000 --- a/api/v1alpha1/group_types_test.go +++ /dev/null @@ -1,194 +0,0 @@ -package v1alpha1 - -import ( - "context" - "encoding/json" - "net/http" - "net/http/httptest" - "testing" - - vault "github.com/hashicorp/vault/api" - vaultutils "github.com/redhat-cop/vault-config-operator/api/v1alpha1/utils" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" -) - -func TestGroupGetPath(t *testing.T) { - tests := []struct { - name string - group *Group - expected string - }{ - { - name: "uses metadata name when spec name is empty", - group: &Group{ - ObjectMeta: metav1.ObjectMeta{Name: "my-group"}, - }, - expected: "identity/group/name/my-group", - }, - { - name: "spec name takes precedence over metadata name", - group: &Group{ - ObjectMeta: metav1.ObjectMeta{Name: "my-group"}, - Spec: GroupSpec{Name: "override-name"}, - }, - expected: "identity/group/name/override-name", - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if got := tt.group.GetPath(); got != tt.expected { - t.Errorf("GetPath() = %q, want %q", got, tt.expected) - } - }) - } -} - -func TestGroupGetPayloadInternalIncludesMemberFields(t *testing.T) { - group := &Group{ - Spec: GroupSpec{ - GroupConfig: GroupConfig{ - Type: "internal", - Policies: []string{"default"}, - MemberGroupIDs: []string{"group-1"}, - MemberEntityIDs: []string{"entity-1"}, - }, - }, - } - payload := group.GetPayload() - if payload["type"] != "internal" { - t.Errorf("expected type 'internal', got %v", payload["type"]) - } - if _, ok := payload["member_group_ids"]; !ok { - t.Error("expected member_group_ids to be present for internal group") - } - if _, ok := payload["member_entity_ids"]; !ok { - t.Error("expected member_entity_ids to be present for internal group") - } -} - -func TestGroupGetPayloadExternalOmitsMemberFields(t *testing.T) { - group := &Group{ - Spec: GroupSpec{ - GroupConfig: GroupConfig{ - Type: "external", - }, - }, - } - payload := group.GetPayload() - if _, ok := payload["member_group_ids"]; ok { - t.Error("expected member_group_ids to be absent for external group") - } - if _, ok := payload["member_entity_ids"]; ok { - t.Error("expected member_entity_ids to be absent for external group") - } -} - -func TestGroupIsEquivalentToDesiredState(t *testing.T) { - group := &Group{ - Spec: GroupSpec{ - GroupConfig: GroupConfig{ - Type: "internal", - Policies: []string{"default"}, - }, - }, - } - if !group.IsEquivalentToDesiredState(group.GetPayload()) { - t.Error("expected group to be equivalent to its own payload") - } -} - -func TestGroupIsEquivalentToDesiredStateMismatch(t *testing.T) { - group := &Group{ - Spec: GroupSpec{ - GroupConfig: GroupConfig{ - Type: "internal", - Policies: []string{"default"}, - }, - }, - } - payload := group.GetPayload() - payload["type"] = "external" - if group.IsEquivalentToDesiredState(payload) { - t.Error("expected group to not be equivalent to payload with different type") - } -} - -// newVaultTestClient creates a vault.Client pointed at the given httptest.Server. -func newVaultTestClient(t *testing.T, srv *httptest.Server) *vault.Client { - t.Helper() - cfg := vault.DefaultConfig() - cfg.Address = srv.URL - client, err := vault.NewClient(cfg) - if err != nil { - t.Fatalf("vault.NewClient: %v", err) - } - client.SetToken("test-token") - return client -} - -func TestGroupEnrichStatus(t *testing.T) { - const wantID = "abc-123-def-456" - - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "application/json") - json.NewEncoder(w).Encode(map[string]any{ - "data": map[string]any{ - "id": wantID, - "name": "my-group", - }, - }) - })) - defer srv.Close() - - group := &Group{ - ObjectMeta: metav1.ObjectMeta{Name: "my-group"}, - } - ctx := vaultutils.ContextWithVaultClient(context.Background(), newVaultTestClient(t, srv)) - - if err := group.EnrichStatus(ctx); err != nil { - t.Fatalf("EnrichStatus() unexpected error: %v", err) - } - if group.Status.ID != wantID { - t.Errorf("Status.ID = %q, want %q", group.Status.ID, wantID) - } -} - -func TestGroupEnrichStatusNoIDField(t *testing.T) { - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "application/json") - json.NewEncoder(w).Encode(map[string]any{ - "data": map[string]any{ - "name": "my-group", // no "id" key - }, - }) - })) - defer srv.Close() - - group := &Group{ - ObjectMeta: metav1.ObjectMeta{Name: "my-group"}, - } - ctx := vaultutils.ContextWithVaultClient(context.Background(), newVaultTestClient(t, srv)) - - if err := group.EnrichStatus(ctx); err != nil { - t.Fatalf("EnrichStatus() unexpected error: %v", err) - } - if group.Status.ID != "" { - t.Errorf("expected Status.ID to remain empty, got %q", group.Status.ID) - } -} - -func TestGroupEnrichStatusVaultError(t *testing.T) { - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - http.Error(w, `{"errors":["internal server error"]}`, http.StatusInternalServerError) - })) - defer srv.Close() - - group := &Group{ - ObjectMeta: metav1.ObjectMeta{Name: "my-group"}, - } - ctx := vaultutils.ContextWithVaultClient(context.Background(), newVaultTestClient(t, srv)) - - if err := group.EnrichStatus(ctx); err == nil { - t.Error("expected error from Vault 500 response, got nil") - } -} From 33748a40a74b76a355d705521cfcdbb9278f9f4b Mon Sep 17 00:00:00 2001 From: Brett Dudo Date: Thu, 2 Jul 2026 12:58:17 -0700 Subject: [PATCH 5/5] test: add EnrichStatus tests to group_test.go --- api/v1alpha1/group_test.go | 86 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 86 insertions(+) diff --git a/api/v1alpha1/group_test.go b/api/v1alpha1/group_test.go index 8c5f44dc..101ace28 100644 --- a/api/v1alpha1/group_test.go +++ b/api/v1alpha1/group_test.go @@ -1,8 +1,14 @@ package v1alpha1 import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" "testing" + vault "github.com/hashicorp/vault/api" + vaultutils "github.com/redhat-cop/vault-config-operator/api/v1alpha1/utils" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -217,3 +223,83 @@ func TestGroupConditions(t *testing.T) { t.Error("expected Group conditions to be set and retrieved") } } + +// newGroupVaultTestClient creates a vault.Client pointed at the given httptest.Server. +func newGroupVaultTestClient(t *testing.T, srv *httptest.Server) *vault.Client { + t.Helper() + cfg := vault.DefaultConfig() + cfg.Address = srv.URL + client, err := vault.NewClient(cfg) + if err != nil { + t.Fatalf("vault.NewClient: %v", err) + } + client.SetToken("test-token") + return client +} + +func TestGroupEnrichStatus(t *testing.T) { + const wantID = "abc-123-def-456" + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode(map[string]any{ + "data": map[string]any{ + "id": wantID, + "name": "my-group", + }, + }) + })) + defer srv.Close() + + group := &Group{ + ObjectMeta: metav1.ObjectMeta{Name: "my-group"}, + } + ctx := vaultutils.ContextWithVaultClient(context.Background(), newGroupVaultTestClient(t, srv)) + + if err := group.EnrichStatus(ctx); err != nil { + t.Fatalf("EnrichStatus() unexpected error: %v", err) + } + if group.Status.ID != wantID { + t.Errorf("Status.ID = %q, want %q", group.Status.ID, wantID) + } +} + +func TestGroupEnrichStatusNoIDField(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode(map[string]any{ + "data": map[string]any{ + "name": "my-group", // no "id" key + }, + }) + })) + defer srv.Close() + + group := &Group{ + ObjectMeta: metav1.ObjectMeta{Name: "my-group"}, + } + ctx := vaultutils.ContextWithVaultClient(context.Background(), newGroupVaultTestClient(t, srv)) + + if err := group.EnrichStatus(ctx); err != nil { + t.Fatalf("EnrichStatus() unexpected error: %v", err) + } + if group.Status.ID != "" { + t.Errorf("expected Status.ID to remain empty, got %q", group.Status.ID) + } +} + +func TestGroupEnrichStatusVaultError(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Error(w, `{"errors":["internal server error"]}`, http.StatusInternalServerError) + })) + defer srv.Close() + + group := &Group{ + ObjectMeta: metav1.ObjectMeta{Name: "my-group"}, + } + ctx := vaultutils.ContextWithVaultClient(context.Background(), newGroupVaultTestClient(t, srv)) + + if err := group.EnrichStatus(ctx); err == nil { + t.Error("expected error from Vault 500 response, got nil") + } +}