Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ require (
github.com/mendsley/gojwk v0.0.0-20141217222730-4d5ec6e58103
github.com/oapi-codegen/runtime v1.6.0
github.com/onsi/gomega v1.42.1
github.com/openshift-hyperfleet/hyperfleet-api-spec v1.0.26
github.com/openshift-hyperfleet/hyperfleet-api-spec v1.0.27
github.com/prometheus/client_golang v1.24.1
github.com/prometheus/client_model v0.6.2
github.com/spf13/cobra v1.10.2
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -190,8 +190,8 @@ github.com/opencontainers/go-digest v1.0.0 h1:apOUWs51W5PlhuyGyz9FCeeBIOUDA/6nW8
github.com/opencontainers/go-digest v1.0.0/go.mod h1:0JzlMkj0TRzQZfJkVvzbP0HBR3IKzErnv2BNG4W4MAM=
github.com/opencontainers/image-spec v1.1.1 h1:y0fUlFfIZhPF1W537XOLg0/fcx6zcHCJwooC2xJA040=
github.com/opencontainers/image-spec v1.1.1/go.mod h1:qpqAh3Dmcf36wStyyWU+kCeDgrGnAve2nCC8+7h8Q0M=
github.com/openshift-hyperfleet/hyperfleet-api-spec v1.0.26 h1:i+hXWKhj/WTuwzAiTzmj+bmRjhJzaByQUOnC68SDnkM=
github.com/openshift-hyperfleet/hyperfleet-api-spec v1.0.26/go.mod h1:KITzIAd8HcMpH5lXdHFjgk45dvL6XLpP3wwz8iK+KCI=
github.com/openshift-hyperfleet/hyperfleet-api-spec v1.0.27 h1:wtLN7KFgsDHaYDBElMcOUjHVeqjRs6dlGOWiobS1/qk=
github.com/openshift-hyperfleet/hyperfleet-api-spec v1.0.27/go.mod h1:KITzIAd8HcMpH5lXdHFjgk45dvL6XLpP3wwz8iK+KCI=
github.com/pelletier/go-toml/v2 v2.4.3 h1:GTRvJQutkOSftxIFD5xw9aepkYNuPWmVJpffdDPYVpY=
github.com/pelletier/go-toml/v2 v2.4.3/go.mod h1:2gIqNv+qfxSVS7cM2xJQKtLSTLUE9V8t9Stt+h56mCY=
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
Expand Down
18 changes: 18 additions & 0 deletions pkg/api/presenters/resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import (
"encoding/json"
"fmt"

"gorm.io/datatypes"

"github.com/openshift-hyperfleet/hyperfleet-api/pkg/api"
"github.com/openshift-hyperfleet/hyperfleet-api/pkg/api/openapi"
"github.com/openshift-hyperfleet/hyperfleet-api/pkg/registry"
Expand Down Expand Up @@ -54,6 +56,7 @@ func PresentResource(r *api.Resource) openapi.Resource {
}

labels := presentLabels(r.Labels)
tenancy := presentTenancy(r.Tenancy)

resp := openapi.Resource{
Id: r.ID,
Expand All @@ -62,6 +65,7 @@ func PresentResource(r *api.Resource) openapi.Resource {
Href: util.PtrString(r.Href),
Spec: spec,
Labels: labels,
Tenancy: tenancy,
Generation: r.Generation,
CreatedTime: r.CreatedTime,
UpdatedTime: r.UpdatedTime,
Expand Down Expand Up @@ -172,3 +176,17 @@ func presentLabels(labels []api.ResourceLabel) *map[string]string {
}
return &m
}

func presentTenancy(t datatypes.JSON) *map[string]string {
if len(t) == 0 {
return nil
}
Comment on lines +180 to +183

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

presentTenancy returns nil for zero-length JSON but {} for stored "{}". Worth normalizing so empty tenancy always serializes the same way.

var m map[string]string
if err := json.Unmarshal(t, &m); err != nil {
return nil
}
if m == nil {
m = map[string]string{}
}
return &m
}
51 changes: 51 additions & 0 deletions pkg/api/presenters/resource_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,57 @@ func TestPresentResource_WithReferences(t *testing.T) {
Expect(*refs["wif_config"][1].Id).To(Equal("wif-2"))
}

func TestPresentResource_WithTenancy(t *testing.T) {
RegisterTestingT(t)

now := time.Now()
resource := &api.Resource{
Meta: api.Meta{ID: "id", CreatedTime: now, UpdatedTime: now},
Kind: "Channel",
Name: "test",
Spec: datatypes.JSON(`{}`),
Tenancy: datatypes.JSON(`{"org":"acme"}`),
CreatedBy: "user@test.com",
UpdatedBy: "user@test.com",
}

resp := PresentResource(resource)
Expect(resp.Tenancy).ToNot(BeNil())
Expect(*resp.Tenancy).To(HaveKeyWithValue("org", "acme"))
}

func TestPresentResource_EmptyTenancy(t *testing.T) {
RegisterTestingT(t)

now := time.Now()
Comment on lines +267 to +289

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider consolidating these into a table-driven test. The cases share the same setup and assertions, and a single table will scale better as we add more tenancy presentation scenarios.

resource := &api.Resource{
Meta: api.Meta{ID: "id", CreatedTime: now, UpdatedTime: now},
Kind: "Channel",
Name: "test",
Spec: datatypes.JSON(`{}`),
Tenancy: datatypes.JSON(`{}`),
CreatedBy: "user@test.com",
UpdatedBy: "user@test.com",
}

resp := PresentResource(resource)
Expect(resp.Tenancy).ToNot(BeNil(), "empty tenancy should present as explicit {}, not be omitted")
Expect(*resp.Tenancy).To(BeEmpty())
}

func TestConvertResource_IgnoresTenancyInBody(t *testing.T) {
RegisterTestingT(t)

body := []byte(`{"kind":"Channel","name":"stable","spec":{"is_default":true},"tenancy":{"org":"forged"}}`)
var req openapi.ResourceCreateRequest
err := json.Unmarshal(body, &req)
Expect(err).NotTo(HaveOccurred())

resource, convErr := ConvertResource(&req)
Expect(convErr).NotTo(HaveOccurred())
Expect(resource.Tenancy).To(BeEmpty())
}

func TestPresentResourceList(t *testing.T) {
RegisterTestingT(t)

Expand Down
2 changes: 2 additions & 0 deletions pkg/handlers/resource_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1005,6 +1005,7 @@ func TestResourceHandler_Patch_RejectsUnknownFields(t *testing.T) {
{"rejects id", `{"id":"some-id","spec":{"is_default":true}}`},
{"rejects generation", `{"generation":5,"spec":{"is_default":true}}`},
{"rejects kind", `{"kind":"Channel","spec":{"is_default":true}}`},
{"rejects tenancy", `{"tenancy":{"org":"acme"},"spec":{"is_default":true}}`},
}

for _, tt := range tests {
Expand Down Expand Up @@ -1080,6 +1081,7 @@ func TestResourceHandler_PatchByOwner_RejectsUnknownFields(t *testing.T) {
{"rejects id", `{"id":"some-id","spec":{"enabled":true}}`},
{"rejects generation", `{"generation":5,"spec":{"enabled":true}}`},
{"rejects kind", `{"kind":"Version","spec":{"enabled":true}}`},
{"rejects tenancy", `{"tenancy":{"org":"acme"},"spec":{"enabled":true}}`},
}

for _, tt := range tests {
Expand Down
2 changes: 2 additions & 0 deletions pkg/services/resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"github.com/openshift-hyperfleet/hyperfleet-api/pkg/logger"
"github.com/openshift-hyperfleet/hyperfleet-api/pkg/metrics"
"github.com/openshift-hyperfleet/hyperfleet-api/pkg/registry"
"github.com/openshift-hyperfleet/hyperfleet-api/pkg/tenant"
"github.com/openshift-hyperfleet/hyperfleet-api/pkg/util"
)

Expand Down Expand Up @@ -130,6 +131,7 @@ func (s *sqlResourceService) Create(
if resource.UpdatedBy == "" {
resource.UpdatedBy = username
}
resource.Tenancy = tenant.TenancyJSON(ctx)

resource, err := s.resourceDao.Create(ctx, resource)
if err != nil {
Expand Down
68 changes: 68 additions & 0 deletions pkg/services/resource_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,15 @@ import (
"time"

. "github.com/onsi/gomega"
"gorm.io/datatypes"
"gorm.io/gorm"

"github.com/openshift-hyperfleet/hyperfleet-api/pkg/api"
"github.com/openshift-hyperfleet/hyperfleet-api/pkg/auth"
"github.com/openshift-hyperfleet/hyperfleet-api/pkg/dao"
"github.com/openshift-hyperfleet/hyperfleet-api/pkg/errors"
"github.com/openshift-hyperfleet/hyperfleet-api/pkg/registry"
"github.com/openshift-hyperfleet/hyperfleet-api/pkg/tenant"
)

const (
Expand Down Expand Up @@ -409,6 +411,55 @@ func TestResourceService_Create_SetsUserFromAuthContext(t *testing.T) {
Expect(result.UpdatedBy).To(Equal("user@test.com"))
}

func TestResourceService_Create_StampsTenancyFromContext(t *testing.T) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider a test that pre-sets resource.Tenancy before calling Create and asserts it is overwritten by TenancyJSON(ctx). Create already does this unconditionally, but it is not covered today.

RegisterTestingT(t)
setupTestDescriptors()

mockDao := newMockResourceDao()
svc, _, _ := newTestResourceService(mockDao)

ctx := tenant.WithTenant(context.Background(), &tenant.ResolvedTenant{
Dimensions: map[string]string{"org": "acme"},
})
resource := testResource("Channel", "ch-1", "stable")

result, svcErr := svc.Create(ctx, "Channel", resource, nil)
Expect(svcErr).To(BeNil())
Expect(string(result.Tenancy)).To(MatchJSON(`{"org":"acme"}`))
}

func TestResourceService_Create_SystemIdentityGetsEmptyTenancy(t *testing.T) {
RegisterTestingT(t)
setupTestDescriptors()

mockDao := newMockResourceDao()
svc, _, _ := newTestResourceService(mockDao)

ctx := tenant.WithTenant(context.Background(), &tenant.ResolvedTenant{
System: true,
Dimensions: map[string]string{"org": "acme"},
})
resource := testResource("Channel", "ch-1", "stable")

result, svcErr := svc.Create(ctx, "Channel", resource, nil)
Expect(svcErr).To(BeNil())
Expect(string(result.Tenancy)).To(MatchJSON(`{}`))
}

func TestResourceService_Create_NoTenantContext_GetsEmptyTenancy(t *testing.T) {
RegisterTestingT(t)
Comment on lines +414 to +450

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider consolidating the three Create tenancy tests into one table-driven test. They share the same setup and only differ by context input and expected Tenancy JSON, similar to TestTenancyJSON in pkg/tenant/context_test.go.

setupTestDescriptors()

mockDao := newMockResourceDao()
svc, _, _ := newTestResourceService(mockDao)

resource := testResource("Channel", "ch-1", "stable")

result, svcErr := svc.Create(context.Background(), "Channel", resource, nil)
Expect(svcErr).To(BeNil())
Expect(string(result.Tenancy)).To(MatchJSON(`{}`))
}

func TestResourceService_Create_PreservesExplicitValues(t *testing.T) {
RegisterTestingT(t)
setupTestDescriptors()
Expand Down Expand Up @@ -564,6 +615,23 @@ func TestResourceService_Patch_SpecChanged_IncrementsGeneration(t *testing.T) {
Expect(result.Generation).To(Equal(int32(2)))
}

func TestResourceService_Patch_DoesNotModifyTenancy(t *testing.T) {
RegisterTestingT(t)
setupTestDescriptors()

mockDao := newMockResourceDao()
svc, _, _ := newTestResourceService(mockDao)

existing := testResource("Channel", "ch-1", "stable")
existing.Tenancy = datatypes.JSON(`{"org":"acme"}`)
mockDao.addResource(existing)

patch := &api.ResourcePatch{Spec: map[string]interface{}{"key": "new-value"}}
result, svcErr := svc.Patch(context.Background(), "Channel", "ch-1", patch)
Expect(svcErr).To(BeNil())
Expect(string(result.Tenancy)).To(MatchJSON(`{"org":"acme"}`))
}

func TestResourceService_Patch_LabelsChanged_IncrementsGeneration(t *testing.T) {
RegisterTestingT(t)
setupTestDescriptors()
Expand Down
43 changes: 43 additions & 0 deletions pkg/tenant/context.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
package tenant

import (
"context"
"encoding/json"

"gorm.io/datatypes"
)

type contextKey struct{}

// ResolvedTenant holds the tenant identity resolved from gateway-injected request headers.
type ResolvedTenant struct {
Dimensions map[string]string
System bool
}

// WithTenant attaches a resolved tenant identity to the context.
func WithTenant(ctx context.Context, t *ResolvedTenant) context.Context {
return context.WithValue(ctx, contextKey{}, t)
}

// FromContext returns the tenant identity attached to ctx, or nil if none was resolved.
func FromContext(ctx context.Context) *ResolvedTenant {
if t, ok := ctx.Value(contextKey{}).(*ResolvedTenant); ok {
return t
}
return nil
}

// TenancyJSON returns the caller's tenancy map as JSONB for storage on created resources.
// System and unscoped/absent callers get an empty map, which no tenant-scoped query can ever match.
func TenancyJSON(ctx context.Context) datatypes.JSON {
t := FromContext(ctx)
if t == nil || t.System || len(t.Dimensions) == 0 {
return datatypes.JSON([]byte("{}"))
}
b, err := json.Marshal(t.Dimensions)
if err != nil {
return datatypes.JSON([]byte("{}"))
}
return datatypes.JSON(b)
}
74 changes: 74 additions & 0 deletions pkg/tenant/context_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
package tenant

import (
"context"
"testing"

. "github.com/onsi/gomega"
)

func TestFromContext_NoTenant(t *testing.T) {
RegisterTestingT(t)

got := FromContext(context.Background())
Expect(got).To(BeNil())
}

func TestWithTenant_RoundTrip(t *testing.T) {
RegisterTestingT(t)

want := &ResolvedTenant{Dimensions: map[string]string{"org": "acme"}}
ctx := WithTenant(context.Background(), want)

got := FromContext(ctx)
Expect(got).To(Equal(want))
}

func TestTenancyJSON(t *testing.T) {
tests := []struct {
name string
ctx context.Context
want string
}{
{
name: "no tenant in context",
ctx: context.Background(),
want: "{}",
},
{
name: "system identity",
ctx: WithTenant(context.Background(), &ResolvedTenant{System: true, Dimensions: map[string]string{"org": "acme"}}),
want: "{}",
},
{
name: "empty dimensions",
ctx: WithTenant(context.Background(), &ResolvedTenant{Dimensions: map[string]string{}}),
want: "{}",
},
{
name: "nil dimensions",
ctx: WithTenant(context.Background(), &ResolvedTenant{}),
want: "{}",
},
{
name: "tenant with single dimension",
ctx: WithTenant(context.Background(), &ResolvedTenant{Dimensions: map[string]string{"org": "acme"}}),
want: `{"org":"acme"}`,
},
{
name: "tenant with multiple dimensions",
ctx: WithTenant(context.Background(), &ResolvedTenant{
Dimensions: map[string]string{"org": "acme", "project": "project-1"},
}),
want: `{"org":"acme","project":"project-1"}`,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
RegisterTestingT(t)
got := TenancyJSON(tt.ctx)
Expect(string(got)).To(MatchJSON(tt.want))
})
}
}