-
Notifications
You must be signed in to change notification settings - Fork 21
HYPERFLEET-1084 - feat: add generic resource data layer with entity registry #180
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,105 @@ | ||
| package api | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "time" | ||
|
|
||
| "gorm.io/datatypes" | ||
| "gorm.io/gorm" | ||
|
|
||
| "github.com/openshift-hyperfleet/hyperfleet-api/pkg/registry" | ||
| ) | ||
|
|
||
| // Resource is the generic GORM model for entity types managed by the entity | ||
| // registry (Channel, Version, WIF Config, etc.). Entity kinds are | ||
| // differentiated by the Kind field. Existing Cluster and NodePool types | ||
| // are NOT migrated to this model. | ||
| type Resource struct { | ||
| Meta | ||
| Kind string `json:"kind" gorm:"size:100;not null"` | ||
| Name string `json:"name" gorm:"size:100;not null"` | ||
| Href string `json:"href,omitempty" gorm:"size:500"` | ||
| CreatedBy string `json:"created_by" gorm:"size:255;not null"` | ||
| UpdatedBy string `json:"updated_by" gorm:"size:255;not null"` | ||
| DeletedBy *string `json:"deleted_by,omitempty" gorm:"size:255"` | ||
| DeletedTime *time.Time `json:"deleted_time,omitempty"` | ||
| OwnerID *string `json:"owner_id,omitempty" gorm:"size:255"` | ||
| OwnerKind *string `json:"owner_kind,omitempty" gorm:"size:100"` | ||
| OwnerHref *string `json:"owner_href,omitempty" gorm:"size:500"` | ||
| Spec datatypes.JSON `json:"spec" gorm:"type:jsonb;not null"` | ||
| Labels datatypes.JSON `json:"labels,omitempty" gorm:"type:jsonb"` | ||
| Generation int32 `json:"generation" gorm:"default:1;not null"` | ||
| } | ||
|
|
||
| type ( | ||
| ResourceList []*Resource | ||
| ResourceIndex map[string]*Resource | ||
| ) | ||
|
|
||
| // TODO: Evaluate the need for this method as part of | ||
| // https://redhat.atlassian.net/browse/HYPERFLEET-1085 and remove if not needed | ||
| func (l ResourceList) Index() ResourceIndex { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Dead code? Appears only in the test itself 🙂
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This follows the ClusterList.Index() pattern — where the service layer (HYPERFLEET-1085) should use it for batch lookups. Included here to keep the data layer's API surface complete. I can add a TODO here to evaluate the need and remove from 1085 if not needed |
||
| index := ResourceIndex{} | ||
| for _, o := range l { | ||
| index[o.ID] = o | ||
| } | ||
| return index | ||
| } | ||
|
|
||
| func (r Resource) TableName() string { | ||
| return "resources" | ||
| } | ||
|
|
||
| // BeforeCreate TODO: Validate the necessity for this as part of https://redhat.atlassian.net/browse/HYPERFLEET-1085 | ||
| func (r *Resource) BeforeCreate(tx *gorm.DB) error { | ||
| if r.ID == "" { | ||
| id, err := NewID() | ||
| if err != nil { | ||
| return fmt.Errorf("failed to generate resource ID: %w", err) | ||
| } | ||
| r.ID = id | ||
| } | ||
|
|
||
| now := time.Now() | ||
| if r.CreatedTime.IsZero() { | ||
| r.CreatedTime = now | ||
| } | ||
| r.UpdatedTime = now | ||
| if r.Generation == 0 { | ||
| r.Generation = 1 | ||
| } | ||
|
|
||
| if r.Href == "" { | ||
| desc := registry.MustGet(r.Kind) | ||
| if r.OwnerID != nil && *r.OwnerID != "" { | ||
| if r.OwnerKind == nil || *r.OwnerKind == "" { | ||
| return fmt.Errorf("owner_kind is required when owner_id is set") | ||
| } | ||
| if r.OwnerHref == nil { | ||
| parentDesc := registry.MustGet(*r.OwnerKind) | ||
| ownerHref := fmt.Sprintf("/api/hyperfleet/v1/%s/%s", | ||
| parentDesc.Plural, *r.OwnerID) | ||
| r.OwnerHref = &ownerHref | ||
| } | ||
| r.Href = fmt.Sprintf("%s/%s/%s", *r.OwnerHref, desc.Plural, r.ID) | ||
| } else { | ||
| r.Href = fmt.Sprintf("/api/hyperfleet/v1/%s/%s", desc.Plural, r.ID) | ||
|
tirthct marked this conversation as resolved.
|
||
| } | ||
|
tirthct marked this conversation as resolved.
|
||
| } | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| func (r *Resource) BeforeUpdate(tx *gorm.DB) error { | ||
| r.UpdatedTime = time.Now() | ||
| return nil | ||
| } | ||
|
|
||
| func (r *Resource) MarkDeleted(by string, t time.Time) { | ||
| r.DeletedTime = &t | ||
| r.DeletedBy = &by | ||
| } | ||
|
|
||
| func (r *Resource) IncrementGeneration() { | ||
| r.Generation++ | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,258 @@ | ||
| package api | ||
|
|
||
| import ( | ||
| "testing" | ||
| "time" | ||
|
|
||
| . "github.com/onsi/gomega" | ||
|
|
||
| "github.com/openshift-hyperfleet/hyperfleet-api/pkg/registry" | ||
| ) | ||
|
|
||
| func setupTestRegistry() { | ||
| registry.Reset() | ||
| registry.Register(registry.EntityDescriptor{ | ||
| Kind: "Channel", | ||
| Plural: "channels", | ||
| }) | ||
| registry.Register(registry.EntityDescriptor{ | ||
| Kind: "Version", | ||
| Plural: "versions", | ||
| ParentKind: "Channel", | ||
| }) | ||
| } | ||
|
|
||
| func strPtr(s string) *string { | ||
| return &s | ||
| } | ||
|
|
||
| func TestResourceList_Index(t *testing.T) { | ||
| RegisterTestingT(t) | ||
|
|
||
| emptyList := ResourceList{} | ||
| emptyIndex := emptyList.Index() | ||
| Expect(len(emptyIndex)).To(Equal(0)) | ||
|
|
||
| r1 := &Resource{} | ||
| r1.ID = "res-1" | ||
| r1.Name = "test-resource-1" | ||
|
|
||
| r2 := &Resource{} | ||
| r2.ID = "res-2" | ||
| r2.Name = "test-resource-2" | ||
|
|
||
| multiList := ResourceList{r1, r2} | ||
| multiIndex := multiList.Index() | ||
| Expect(len(multiIndex)).To(Equal(2)) | ||
| Expect(multiIndex["res-1"]).To(Equal(r1)) | ||
| Expect(multiIndex["res-2"]).To(Equal(r2)) | ||
|
|
||
| r1Dup := &Resource{} | ||
| r1Dup.ID = "res-1" | ||
| r1Dup.Name = "duplicate" | ||
|
|
||
| dupList := ResourceList{r1, r1Dup} | ||
| dupIndex := dupList.Index() | ||
| Expect(len(dupIndex)).To(Equal(1)) | ||
| Expect(dupIndex["res-1"].Name).To(Equal("duplicate")) | ||
| } | ||
|
|
||
| func TestResource_BeforeCreate_IDGeneration(t *testing.T) { | ||
| RegisterTestingT(t) | ||
| setupTestRegistry() | ||
|
|
||
| r := &Resource{Name: "test", Kind: "Channel"} | ||
|
|
||
| err := r.BeforeCreate(nil) | ||
| Expect(err).To(BeNil()) | ||
| Expect(r.ID).ToNot(BeEmpty()) | ||
| } | ||
|
|
||
| func TestResource_BeforeCreate_IDPreservation(t *testing.T) { | ||
| RegisterTestingT(t) | ||
| setupTestRegistry() | ||
|
|
||
| r := &Resource{Name: "test", Kind: "Channel"} | ||
| r.ID = "pre-set-id" | ||
|
|
||
| err := r.BeforeCreate(nil) | ||
| Expect(err).To(BeNil()) | ||
| Expect(r.ID).To(Equal("pre-set-id")) | ||
| } | ||
|
|
||
| func TestResource_BeforeCreate_GenerationDefault(t *testing.T) { | ||
| RegisterTestingT(t) | ||
| setupTestRegistry() | ||
|
|
||
| r := &Resource{Name: "test", Kind: "Channel"} | ||
|
|
||
| err := r.BeforeCreate(nil) | ||
| Expect(err).To(BeNil()) | ||
| Expect(r.Generation).To(Equal(int32(1))) | ||
| } | ||
|
|
||
| func TestResource_BeforeCreate_GenerationPreserved(t *testing.T) { | ||
| RegisterTestingT(t) | ||
| setupTestRegistry() | ||
|
|
||
| r := &Resource{Name: "test", Kind: "Channel", Generation: 5} | ||
|
|
||
| err := r.BeforeCreate(nil) | ||
| Expect(err).To(BeNil()) | ||
| Expect(r.Generation).To(Equal(int32(5))) | ||
| } | ||
|
|
||
| func TestResource_BeforeCreate_Timestamps(t *testing.T) { | ||
| RegisterTestingT(t) | ||
| setupTestRegistry() | ||
|
|
||
| before := time.Now() | ||
| r := &Resource{Name: "test", Kind: "Channel"} | ||
|
|
||
| err := r.BeforeCreate(nil) | ||
| Expect(err).To(BeNil()) | ||
|
|
||
| Expect(r.CreatedTime).ToNot(BeZero()) | ||
| Expect(r.UpdatedTime).ToNot(BeZero()) | ||
| Expect(r.CreatedTime.After(before) || r.CreatedTime.Equal(before)).To(BeTrue()) | ||
| Expect(r.UpdatedTime.After(before) || r.UpdatedTime.Equal(before)).To(BeTrue()) | ||
| } | ||
|
|
||
| func TestResource_BeforeCreate_CreatedTimePreserved(t *testing.T) { | ||
| RegisterTestingT(t) | ||
| setupTestRegistry() | ||
|
|
||
| fixedTime := time.Date(2025, 1, 1, 0, 0, 0, 0, time.UTC) | ||
| r := &Resource{Name: "test", Kind: "Channel"} | ||
| r.CreatedTime = fixedTime | ||
|
|
||
| err := r.BeforeCreate(nil) | ||
| Expect(err).To(BeNil()) | ||
| Expect(r.CreatedTime).To(Equal(fixedTime)) | ||
| } | ||
|
|
||
| func TestResource_BeforeCreate_HrefTopLevel(t *testing.T) { | ||
| RegisterTestingT(t) | ||
| setupTestRegistry() | ||
|
|
||
| r := &Resource{Name: "stable", Kind: "Channel"} | ||
| err := r.BeforeCreate(nil) | ||
| Expect(err).To(BeNil()) | ||
| Expect(r.Href).To(Equal("/api/hyperfleet/v1/channels/" + r.ID)) | ||
| } | ||
|
|
||
| func TestResource_BeforeCreate_HrefChild(t *testing.T) { | ||
| RegisterTestingT(t) | ||
| setupTestRegistry() | ||
|
|
||
| r := &Resource{ | ||
| Name: "4-17-12", | ||
| Kind: "Version", | ||
| OwnerID: strPtr("ch-1"), | ||
| OwnerKind: strPtr("Channel"), | ||
| } | ||
| err := r.BeforeCreate(nil) | ||
| Expect(err).To(BeNil()) | ||
| Expect(r.Href).To(Equal("/api/hyperfleet/v1/channels/ch-1/versions/" + r.ID)) | ||
| Expect(*r.OwnerHref).To(Equal("/api/hyperfleet/v1/channels/ch-1")) | ||
| } | ||
|
|
||
| func TestResource_BeforeCreate_OwnerKindMissing(t *testing.T) { | ||
| RegisterTestingT(t) | ||
| setupTestRegistry() | ||
|
|
||
| r := &Resource{ | ||
| Name: "4-17-12", | ||
| Kind: "Version", | ||
| OwnerID: strPtr("ch-1"), | ||
| } | ||
| err := r.BeforeCreate(nil) | ||
| Expect(err).ToNot(BeNil()) | ||
| Expect(err.Error()).To(ContainSubstring("owner_kind is required")) | ||
| } | ||
|
|
||
| func TestResource_BeforeCreate_OwnerKindEmpty(t *testing.T) { | ||
| RegisterTestingT(t) | ||
| setupTestRegistry() | ||
|
|
||
| r := &Resource{ | ||
| Name: "4-17-12", | ||
| Kind: "Version", | ||
| OwnerID: strPtr("ch-1"), | ||
| OwnerKind: strPtr(""), | ||
| } | ||
| err := r.BeforeCreate(nil) | ||
| Expect(err).ToNot(BeNil()) | ||
| Expect(err.Error()).To(ContainSubstring("owner_kind is required")) | ||
| } | ||
|
|
||
| func TestResource_BeforeCreate_HrefChildWithPresetOwnerHref(t *testing.T) { | ||
| RegisterTestingT(t) | ||
| setupTestRegistry() | ||
|
|
||
| r := &Resource{ | ||
| Name: "some-label", | ||
| Kind: "Version", | ||
| OwnerID: strPtr("v-1"), | ||
| OwnerKind: strPtr("Version"), | ||
| OwnerHref: strPtr("/api/hyperfleet/v1/channels/ch-1/versions/v-1"), | ||
| } | ||
| err := r.BeforeCreate(nil) | ||
| Expect(err).To(BeNil()) | ||
| Expect(r.Href).To(Equal("/api/hyperfleet/v1/channels/ch-1/versions/v-1/versions/" + r.ID)) | ||
| Expect(*r.OwnerHref).To(Equal("/api/hyperfleet/v1/channels/ch-1/versions/v-1")) | ||
| } | ||
|
tirthct marked this conversation as resolved.
|
||
|
|
||
| func TestResource_BeforeCreate_HrefPreserved(t *testing.T) { | ||
| RegisterTestingT(t) | ||
| setupTestRegistry() | ||
|
|
||
| r := &Resource{Name: "test", Kind: "Channel", Href: "/custom/href"} | ||
| err := r.BeforeCreate(nil) | ||
| Expect(err).To(BeNil()) | ||
| Expect(r.Href).To(Equal("/custom/href")) | ||
| } | ||
|
tirthct marked this conversation as resolved.
|
||
|
|
||
| func TestResource_BeforeUpdate_UpdatesTimestamp(t *testing.T) { | ||
| RegisterTestingT(t) | ||
|
|
||
| r := &Resource{Name: "test", Kind: "Channel"} | ||
| r.UpdatedTime = time.Date(2020, 1, 1, 0, 0, 0, 0, time.UTC) | ||
|
|
||
| before := time.Now() | ||
| err := r.BeforeUpdate(nil) | ||
| Expect(err).To(BeNil()) | ||
| Expect(r.UpdatedTime.After(before) || r.UpdatedTime.Equal(before)).To(BeTrue()) | ||
| } | ||
|
|
||
| func TestResource_MarkDeleted(t *testing.T) { | ||
| RegisterTestingT(t) | ||
|
|
||
| r := &Resource{Name: "test", Kind: "Channel"} | ||
| now := time.Now() | ||
|
|
||
| r.MarkDeleted("admin", now) | ||
|
|
||
| Expect(r.DeletedTime).ToNot(BeNil()) | ||
| Expect(*r.DeletedTime).To(Equal(now)) | ||
| Expect(r.DeletedBy).ToNot(BeNil()) | ||
| Expect(*r.DeletedBy).To(Equal("admin")) | ||
| } | ||
|
|
||
| func TestResource_IncrementGeneration(t *testing.T) { | ||
| RegisterTestingT(t) | ||
|
|
||
| r := &Resource{Name: "test", Kind: "Channel", Generation: 1} | ||
| r.IncrementGeneration() | ||
| Expect(r.Generation).To(Equal(int32(2))) | ||
|
|
||
| r.IncrementGeneration() | ||
| Expect(r.Generation).To(Equal(int32(3))) | ||
| } | ||
|
|
||
| func TestResource_TableName(t *testing.T) { | ||
| RegisterTestingT(t) | ||
|
|
||
| r := Resource{} | ||
| Expect(r.TableName()).To(Equal("resources")) | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should be string
https://github.com/rh-amarin/hyperfleet-architecture/blob/a587810415cb18b896f450a2b1890dddad75a529/hyperfleet/docs/generic-resource-registry-design.md#41-gorm-types-pkgapiresourcego
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The DDR uses string, but the implementation uses *string because the partial unique indexes depend on owner_id IS NULL vs IS NOT NULL to separate top-level from child uniqueness constraints. If we replace NULL with "" in SQL, it could lead to non-ideiomatic SQL. Wdyt?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do not take the DDR as definitive, and maybe it should have stayed more "high level" to avoid doing coding decisions too early without testing.
I think the NULL is more idiomatic to mean there is no owner