Skip to content

Propagate JSON unmarshal errors in type conversion functions #24

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.

Sign up for GitHub

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

Merged
openshift-merge-bot merged 1 commit into from
Dec 19, 2025
Merged

Propagate JSON unmarshal errors in type conversion functions #24

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
16 changes: 11 additions & 5 deletions pkg/api/presenters/adapter_status.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,11 +73,13 @@ func ConvertAdapterStatus(
}

// PresentAdapterStatus converts api.AdapterStatus (GORM model) to openapi.AdapterStatus
func PresentAdapterStatus(adapterStatus *api.AdapterStatus) openapi.AdapterStatus {
func PresentAdapterStatus(adapterStatus *api.AdapterStatus) (openapi.AdapterStatus, error) {
// Unmarshal Conditions
var conditions []api.AdapterCondition
if len(adapterStatus.Conditions) > 0 {
_ = json.Unmarshal(adapterStatus.Conditions, &conditions)
if err := json.Unmarshal(adapterStatus.Conditions, &conditions); err != nil {
return openapi.AdapterStatus{}, fmt.Errorf("failed to unmarshal adapter status conditions: %w", err)
}
}

// Convert domain AdapterConditions to openapi format
Expand All @@ -95,13 +97,17 @@ func PresentAdapterStatus(adapterStatus *api.AdapterStatus) openapi.AdapterStatu
// Unmarshal Data
var data map[string]map[string]interface{}
if len(adapterStatus.Data) > 0 {
_ = json.Unmarshal(adapterStatus.Data, &data)
if err := json.Unmarshal(adapterStatus.Data, &data); err != nil {
return openapi.AdapterStatus{}, fmt.Errorf("failed to unmarshal adapter status data: %w", err)
}
}

// Unmarshal Metadata
var metadata *openapi.AdapterStatusBaseMetadata
if len(adapterStatus.Metadata) > 0 {
_ = json.Unmarshal(adapterStatus.Metadata, &metadata)
if err := json.Unmarshal(adapterStatus.Metadata, &metadata); err != nil {
return openapi.AdapterStatus{}, fmt.Errorf("failed to unmarshal adapter status metadata: %w", err)
}
}

// Set default times if nil (shouldn't happen in normal operation)
Expand All @@ -123,5 +129,5 @@ func PresentAdapterStatus(adapterStatus *api.AdapterStatus) openapi.AdapterStatu
Metadata: metadata,
CreatedTime: createdTime,
LastReportTime: lastReportTime,
}
}, nil
}
88 changes: 82 additions & 6 deletions pkg/api/presenters/adapter_status_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,8 @@ func TestPresentAdapterStatus_Complete(t *testing.T) {
LastReportTime: &now,
}

result := PresentAdapterStatus(adapterStatus)
result, err := PresentAdapterStatus(adapterStatus)
Expect(err).To(BeNil())

// Verify basic fields
Expect(result.Adapter).To(Equal("aws-adapter"))
Expand Down Expand Up @@ -305,7 +306,8 @@ func TestPresentAdapterStatus_NilTimestamps(t *testing.T) {
LastReportTime: nil, // Nil timestamp
}

result := PresentAdapterStatus(adapterStatus)
result, err := PresentAdapterStatus(adapterStatus)
Expect(err).To(BeNil())

// Verify zero time.Time is returned (not nil)
Expect(result.CreatedTime.IsZero()).To(BeTrue())
Expand All @@ -328,7 +330,8 @@ func TestPresentAdapterStatus_EmptyConditions(t *testing.T) {
LastReportTime: &now,
}

result := PresentAdapterStatus(adapterStatus)
result, err := PresentAdapterStatus(adapterStatus)
Expect(err).To(BeNil())

Expect(result.Conditions).ToNot(BeNil())
Expect(len(result.Conditions)).To(Equal(0))
Expand All @@ -350,7 +353,8 @@ func TestPresentAdapterStatus_EmptyData(t *testing.T) {
LastReportTime: &now,
}

result := PresentAdapterStatus(adapterStatus)
result, err := PresentAdapterStatus(adapterStatus)
Expect(err).To(BeNil())

Expect(result.Data).ToNot(BeNil())
Expect(len(result.Data)).To(Equal(0))
Expand Down Expand Up @@ -391,7 +395,8 @@ func TestPresentAdapterStatus_ConditionStatusConversion(t *testing.T) {
LastReportTime: &now,
}

result := PresentAdapterStatus(adapterStatus)
result, err := PresentAdapterStatus(adapterStatus)
Expect(err).To(BeNil())

Expect(len(result.Conditions)).To(Equal(1))
Expect(result.Conditions[0].Status).To(Equal(tc.expectedOpenAPI))
Expand All @@ -409,7 +414,8 @@ func TestConvertAndPresent_RoundTrip(t *testing.T) {
Expect(err).To(BeNil())

// Present from domain back to OpenAPI
result := PresentAdapterStatus(adapterStatus)
result, err := PresentAdapterStatus(adapterStatus)
Expect(err).To(BeNil())

// Verify data integrity
Expect(result.Adapter).To(Equal(originalReq.Adapter))
Expand All @@ -429,3 +435,73 @@ func TestConvertAndPresent_RoundTrip(t *testing.T) {
Expect(result.Metadata).ToNot(BeNil())
Expect(*result.Metadata.JobName).To(Equal(*originalReq.Metadata.JobName))
}

// TestPresentAdapterStatus_MalformedConditions tests error handling for malformed Conditions JSON
func TestPresentAdapterStatus_MalformedConditions(t *testing.T) {
RegisterTestingT(t)

now := time.Now()
adapterStatus := &api.AdapterStatus{
ResourceType: "Cluster",
ResourceID: "cluster-123",
Adapter: "test-adapter",
ObservedGeneration: 5,
Conditions: []byte("{invalid json}"), // Malformed JSON
Data: []byte("{}"),
CreatedTime: &now,
LastReportTime: &now,
}
adapterStatus.ID = "adapter-status-malformed-conditions"

_, err := PresentAdapterStatus(adapterStatus)

Expect(err).ToNot(BeNil())
Expect(err.Error()).To(ContainSubstring("failed to unmarshal adapter status conditions"))
}

// TestPresentAdapterStatus_MalformedData tests error handling for malformed Data JSON
func TestPresentAdapterStatus_MalformedData(t *testing.T) {
RegisterTestingT(t)

now := time.Now()
adapterStatus := &api.AdapterStatus{
ResourceType: "Cluster",
ResourceID: "cluster-456",
Adapter: "test-adapter",
ObservedGeneration: 5,
Conditions: []byte("[]"),
Data: []byte("{not valid json"), // Malformed JSON
CreatedTime: &now,
LastReportTime: &now,
}
adapterStatus.ID = "adapter-status-malformed-data"

_, err := PresentAdapterStatus(adapterStatus)

Expect(err).ToNot(BeNil())
Expect(err.Error()).To(ContainSubstring("failed to unmarshal adapter status data"))
}

// TestPresentAdapterStatus_MalformedMetadata tests error handling for malformed Metadata JSON
func TestPresentAdapterStatus_MalformedMetadata(t *testing.T) {
RegisterTestingT(t)

now := time.Now()
adapterStatus := &api.AdapterStatus{
ResourceType: "Cluster",
ResourceID: "cluster-789",
Adapter: "test-adapter",
ObservedGeneration: 5,
Conditions: []byte("[]"),
Data: []byte("{}"),
Metadata: []byte("[{incomplete"), // Malformed JSON
CreatedTime: &now,
LastReportTime: &now,
}
adapterStatus.ID = "adapter-status-malformed-metadata"

_, err := PresentAdapterStatus(adapterStatus)

Expect(err).ToNot(BeNil())
Expect(err.Error()).To(ContainSubstring("failed to unmarshal adapter status metadata"))
}
16 changes: 11 additions & 5 deletions pkg/api/presenters/cluster.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,23 +48,29 @@ func ConvertCluster(req *openapi.ClusterCreateRequest, createdBy string) (*api.C
}

// PresentCluster converts api.Cluster (GORM model) to openapi.Cluster
func PresentCluster(cluster *api.Cluster) openapi.Cluster {
func PresentCluster(cluster *api.Cluster) (openapi.Cluster, error) {
// Unmarshal Spec
var spec map[string]interface{}
if len(cluster.Spec) > 0 {
_ = json.Unmarshal(cluster.Spec, &spec)
if err := json.Unmarshal(cluster.Spec, &spec); err != nil {
return openapi.Cluster{}, fmt.Errorf("failed to unmarshal cluster spec: %w", err)
}
}

// Unmarshal Labels
var labels map[string]string
if len(cluster.Labels) > 0 {
_ = json.Unmarshal(cluster.Labels, &labels)
if err := json.Unmarshal(cluster.Labels, &labels); err != nil {
return openapi.Cluster{}, fmt.Errorf("failed to unmarshal cluster labels: %w", err)
}
}

// Unmarshal StatusConditions
var statusConditions []api.ResourceCondition
if len(cluster.StatusConditions) > 0 {
_ = json.Unmarshal(cluster.StatusConditions, &statusConditions)
if err := json.Unmarshal(cluster.StatusConditions, &statusConditions); err != nil {
return openapi.Cluster{}, fmt.Errorf("failed to unmarshal cluster status conditions: %w", err)
}
}

// Generate Href if not set (fallback)
Expand Down Expand Up @@ -127,5 +133,5 @@ func PresentCluster(cluster *api.Cluster) openapi.Cluster {
LastUpdatedTime: lastUpdatedTime,
}

return result
return result, nil
}
75 changes: 69 additions & 6 deletions pkg/api/presenters/cluster_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,8 @@ func TestPresentCluster_Complete(t *testing.T) {
cluster.CreatedTime = now
cluster.UpdatedTime = now

result := PresentCluster(cluster)
result, err := PresentCluster(cluster)
Expect(err).To(BeNil())

// Verify basic fields
Expect(*result.Id).To(Equal("cluster-abc123"))
Expand Down Expand Up @@ -249,7 +250,8 @@ func TestPresentCluster_HrefGeneration(t *testing.T) {
}
cluster.ID = "cluster-xyz789"

result := PresentCluster(cluster)
result, err := PresentCluster(cluster)
Expect(err).To(BeNil())

Expect(*result.Href).To(Equal("/api/hyperfleet/v1/clusters/cluster-xyz789"))
}
Expand All @@ -268,7 +270,8 @@ func TestPresentCluster_EmptyStatusPhase(t *testing.T) {
}
cluster.ID = "cluster-empty-phase"

result := PresentCluster(cluster)
result, err := PresentCluster(cluster)
Expect(err).To(BeNil())

// Should use NOT_READY as default
Expect(result.Status.Phase).To(Equal(openapi.NOT_READY))
Expand All @@ -292,7 +295,8 @@ func TestPresentCluster_NilStatusTimestamps(t *testing.T) {
cluster.CreatedTime = now
cluster.UpdatedTime = now

result := PresentCluster(cluster)
result, err := PresentCluster(cluster)
Expect(err).To(BeNil())

// Verify zero time.Time is returned (not nil)
Expect(result.Status.LastTransitionTime.IsZero()).To(BeTrue())
Expand Down Expand Up @@ -345,7 +349,8 @@ func TestPresentCluster_StatusConditionsConversion(t *testing.T) {
cluster.CreatedTime = now
cluster.UpdatedTime = now

result := PresentCluster(cluster)
result, err := PresentCluster(cluster)
Expect(err).To(BeNil())

// Verify both conditions converted correctly
Expect(len(result.Status.Conditions)).To(Equal(2))
Expand Down Expand Up @@ -380,7 +385,8 @@ func TestConvertAndPresentCluster_RoundTrip(t *testing.T) {
cluster.UpdatedTime = now

// Present from domain back to OpenAPI
result := PresentCluster(cluster)
result, err := PresentCluster(cluster)
Expect(err).To(BeNil())

// Verify data integrity
Expect(*result.Id).To(Equal("cluster-roundtrip-123"))
Expand All @@ -401,3 +407,60 @@ func TestConvertAndPresentCluster_RoundTrip(t *testing.T) {
Expect(result.Status.ObservedGeneration).To(Equal(int32(0)))
Expect(len(result.Status.Conditions)).To(Equal(0))
}

// TestPresentCluster_MalformedSpec tests error handling for malformed Spec JSON
func TestPresentCluster_MalformedSpec(t *testing.T) {
RegisterTestingT(t)

cluster := &api.Cluster{
Kind: "Cluster",
Name: "malformed-spec-cluster",
Spec: []byte("{invalid json}"), // Malformed JSON
Labels: []byte("{}"),
StatusConditions: []byte("[]"),
}
cluster.ID = "cluster-malformed-spec"

_, err := PresentCluster(cluster)

Expect(err).ToNot(BeNil())
Expect(err.Error()).To(ContainSubstring("failed to unmarshal cluster spec"))
}

// TestPresentCluster_MalformedLabels tests error handling for malformed Labels JSON
func TestPresentCluster_MalformedLabels(t *testing.T) {
RegisterTestingT(t)

cluster := &api.Cluster{
Kind: "Cluster",
Name: "malformed-labels-cluster",
Spec: []byte("{}"),
Labels: []byte("{not valid json"), // Malformed JSON
StatusConditions: []byte("[]"),
}
cluster.ID = "cluster-malformed-labels"

_, err := PresentCluster(cluster)

Expect(err).ToNot(BeNil())
Expect(err.Error()).To(ContainSubstring("failed to unmarshal cluster labels"))
}

// TestPresentCluster_MalformedStatusConditions tests error handling for malformed StatusConditions JSON
func TestPresentCluster_MalformedStatusConditions(t *testing.T) {
RegisterTestingT(t)

cluster := &api.Cluster{
Kind: "Cluster",
Name: "malformed-conditions-cluster",
Spec: []byte("{}"),
Labels: []byte("{}"),
StatusConditions: []byte("[{incomplete"), // Malformed JSON
}
cluster.ID = "cluster-malformed-conditions"

_, err := PresentCluster(cluster)

Expect(err).ToNot(BeNil())
Expect(err.Error()).To(ContainSubstring("failed to unmarshal cluster status conditions"))
}
16 changes: 11 additions & 5 deletions pkg/api/presenters/node_pool.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,23 +54,29 @@ func ConvertNodePool(req *openapi.NodePoolCreateRequest, ownerID, createdBy stri
}

// PresentNodePool converts api.NodePool (GORM model) to openapi.NodePool
func PresentNodePool(nodePool *api.NodePool) openapi.NodePool {
func PresentNodePool(nodePool *api.NodePool) (openapi.NodePool, error) {
// Unmarshal Spec
var spec map[string]interface{}
if len(nodePool.Spec) > 0 {
_ = json.Unmarshal(nodePool.Spec, &spec)
if err := json.Unmarshal(nodePool.Spec, &spec); err != nil {
return openapi.NodePool{}, fmt.Errorf("failed to unmarshal nodepool spec: %w", err)
}
}

// Unmarshal Labels
var labels map[string]string
if len(nodePool.Labels) > 0 {
_ = json.Unmarshal(nodePool.Labels, &labels)
if err := json.Unmarshal(nodePool.Labels, &labels); err != nil {
return openapi.NodePool{}, fmt.Errorf("failed to unmarshal nodepool labels: %w", err)
}
}

// Unmarshal StatusConditions
var statusConditions []api.ResourceCondition
if len(nodePool.StatusConditions) > 0 {
_ = json.Unmarshal(nodePool.StatusConditions, &statusConditions)
if err := json.Unmarshal(nodePool.StatusConditions, &statusConditions); err != nil {
return openapi.NodePool{}, fmt.Errorf("failed to unmarshal nodepool status conditions: %w", err)
}
}

// Generate Href if not set (fallback)
Expand Down Expand Up @@ -145,5 +151,5 @@ func PresentNodePool(nodePool *api.NodePool) openapi.NodePool {
LastUpdatedTime: lastUpdatedTime,
}

return result
return result, nil
}
Loading