Skip to content

fix(datareading): improve DataReading JSON parsing and error handling #710

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
wallrj-cyberark merged 1 commit into from
Sep 5, 2025
Merged

fix(datareading): improve DataReading JSON parsing and error handling #710

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
65 changes: 36 additions & 29 deletions api/datareading.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,10 @@ type DataReading struct {
}

// UnmarshalJSON implements the json.Unmarshaler interface for DataReading.
// It handles the dynamic parsing of the Data field based on the DataGatherer.
// The function attempts to decode the Data field into known types in a prioritized order.
// Empty data is considered an error, because there is no way to discriminate between data types.
// TODO(wallrj): Add a discriminator field to DataReading to avoid this complex logic.
// E.g. "data_type": "discovery"|"dynamic"
func (o *DataReading) UnmarshalJSON(data []byte) error {
var tmp struct {
ClusterID string `json:"cluster_id,omitempty"`
Expand All @@ -40,45 +43,49 @@ func (o *DataReading) UnmarshalJSON(data []byte) error {
SchemaVersion string `json:"schema_version"`
}

d := json.NewDecoder(bytes.NewReader(data))
d.DisallowUnknownFields()

if err := d.Decode(&tmp); err != nil {
return err
// Decode the top-level fields of DataReading
if err := jsonUnmarshalStrict(data, &tmp); err != nil {
return fmt.Errorf("failed to parse DataReading: %s", err)
}

// Assign top-level fields to the DataReading object
o.ClusterID = tmp.ClusterID
o.DataGatherer = tmp.DataGatherer
o.Timestamp = tmp.Timestamp
o.SchemaVersion = tmp.SchemaVersion

{
var discoveryData DiscoveryData
d := json.NewDecoder(bytes.NewReader(tmp.Data))
d.DisallowUnknownFields()
if err := d.Decode(&discoveryData); err == nil {
o.Data = &discoveryData
return nil
}
// Return an error if data is empty
if len(tmp.Data) == 0 || bytes.Equal(tmp.Data, []byte("null")) || bytes.Equal(tmp.Data, []byte("{}")) {
return fmt.Errorf("failed to parse DataReading.Data for gatherer %q: empty data", o.DataGatherer)
}
{
var dynamicData DynamicData
d := json.NewDecoder(bytes.NewReader(tmp.Data))
d.DisallowUnknownFields()
if err := d.Decode(&dynamicData); err == nil {
o.Data = &dynamicData
return nil
}

// Define a list of decoding attempts with prioritized types
dataTypes := []struct {
target interface{}
assign func(interface{})
}{
{&DiscoveryData{}, func(v interface{}) { o.Data = v.(*DiscoveryData) }},
{&DynamicData{}, func(v interface{}) { o.Data = v.(*DynamicData) }},
}
{
var genericData map[string]interface{}
d := json.NewDecoder(bytes.NewReader(tmp.Data))
d.DisallowUnknownFields()
if err := d.Decode(&genericData); err == nil {
o.Data = genericData

// Attempt to decode the Data field into each type
for _, dataType := range dataTypes {
if err := jsonUnmarshalStrict(tmp.Data, dataType.target); err == nil {
dataType.assign(dataType.target)
return nil
}
}
return fmt.Errorf("failed to parse DataReading.Data for gatherer %s", o.DataGatherer)

// Return an error if no type matches
return fmt.Errorf("failed to parse DataReading.Data for gatherer %q: unknown type", o.DataGatherer)
}

// jsonUnmarshalStrict unmarshals JSON data into the provided interface,
// disallowing unknown fields to ensure strict adherence to the expected structure.
func jsonUnmarshalStrict(data []byte, v interface{}) error {
decoder := json.NewDecoder(bytes.NewReader(data))
decoder.DisallowUnknownFields()
return decoder.Decode(v)
}

// GatheredResource wraps the raw k8s resource that is sent to the jetstack secure backend
Expand Down
177 changes: 177 additions & 0 deletions api/datareading_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import (
"encoding/json"
"testing"
"time"

"github.com/stretchr/testify/assert"
)

func TestJSONGatheredResourceDropsEmptyTime(t *testing.T) {
Expand Down Expand Up @@ -34,3 +36,178 @@ func TestJSONGatheredResourceSetsTimeWhenPresent(t *testing.T) {
t.Fatalf("unexpected json \ngot %s\nwant %s", string(bytes), expected)
}
}

// TestDataReading_UnmarshalJSON tests the UnmarshalJSON method of DataReading
// with various scenarios including valid and invalid JSON inputs.
func TestDataReading_UnmarshalJSON(t *testing.T) {
tests := []struct {
name string
input string
wantDataType interface{}
expectError string
}{
{
name: "DiscoveryData type",
input: `{
"cluster_id": "61b2db64-fd70-49a6-a257-08397b9b4bae",
"data-gatherer": "discovery",
"timestamp": "2024-06-01T12:00:00Z",
"data": {
"cluster_id": "60868ebf-6e47-4184-9bc0-20bb6824e210",
"server_version": {
"major": "1",
"minor": "20",
"gitVersion": "v1.20.0"
}
},
"schema_version": "v1"
}`,
wantDataType: &DiscoveryData{},
},
{
name: "DynamicData type",
input: `{
"cluster_id": "69050b54-c61a-4384-95c3-35f890377a67",
"data-gatherer": "dynamic",
"timestamp": "2024-06-01T12:00:00Z",
"data": {"items": []},
"schema_version": "v1"
}`,
wantDataType: &DynamicData{},
},
{
name: "Invalid JSON",
input: `not a json`,
expectError: "failed to parse DataReading: invalid character 'o' in literal null (expecting 'u')",
},
{
name: "Missing data field",
input: `{
"cluster_id": "cc5a0429-8dc4-42c8-8e3a-eece9bca15c3",
"data-gatherer": "missing-data-field",
"timestamp": "2024-06-01T12:00:00Z",
"schema_version": "v1"
}`,
expectError: `failed to parse DataReading.Data for gatherer "missing-data-field": empty data`,
},
{
name: "Mismatched data type",
input: `{
"cluster_id": "c272b13e-b19e-4782-833f-d55a305f3c9e",
"data-gatherer": "unknown-data-type",
"timestamp": "2024-06-01T12:00:00Z",
"data": "this should be an object",
"schema_version": "v1"
}`,
expectError: `failed to parse DataReading.Data for gatherer "unknown-data-type": unknown type`,
},
{
name: "Empty data field",
input: `{
"cluster_id": "07909675-113f-4b59-ba5e-529571a191e6",
"data-gatherer": "empty-data",
"timestamp": "2024-06-01T12:00:00Z",
"data": {},
"schema_version": "v1"
}`,
expectError: `failed to parse DataReading.Data for gatherer "empty-data": empty data`,
},
{
name: "Additional field",
input: `{
"cluster_id": "11df7332-4b32-4f5a-903b-0cbbef381850",
"data-gatherer": "additional-field",
"timestamp": "2024-06-01T12:00:00Z",
"data": {
"cluster_id": "60868ebf-6e47-4184-9bc0-20bb6824e210"
},
"extra_field": "should cause error",
"schema_version": "v1"
}`,
expectError: `failed to parse DataReading: json: unknown field "extra_field"`,
},
{
name: "Additional data field",
input: `{
"cluster_id": "ca44c338-987e-4d57-8320-63f538db4292",
"data-gatherer": "additional-data-field",
"timestamp": "2024-06-01T12:00:00Z",
"data": {
"cluster_id": "60868ebf-6e47-4184-9bc0-20bb6824e210",
"server_version": {
"major": "1",
"minor": "20",
"gitVersion": "v1.20.0"
},
"extra_field": "should cause error"
},
"schema_version": "v1"
}`,
expectError: `failed to parse DataReading.Data for gatherer "additional-data-field": unknown type`,
},
{
name: "Empty JSON object",
input: `{}`,
expectError: `failed to parse DataReading.Data for gatherer "": empty data`,
},
{
name: "Null data field",
input: `{
"cluster_id": "36281cb3-7f3a-4efa-9879-7c988a9715b0",
"data-gatherer": "null-data",
"timestamp": "2024-06-01T12:00:00Z",
"data": null,
"schema_version": "v1"
}`,
expectError: `failed to parse DataReading.Data for gatherer "null-data": empty data`,
},
{
name: "Empty string data field",
input: `{
"cluster_id": "7b7aa8ee-58ac-4818-9b29-c0a76296ea1d",
"data-gatherer": "empty-string-data",
"timestamp": "2024-06-01T12:00:00Z",
"data": "",
"schema_version": "v1"
}`,
expectError: `failed to parse DataReading.Data for gatherer "empty-string-data": unknown type`,
},
{
name: "Array instead of object in data field",
input: `{
"cluster_id": "94d7757f-d084-4ccb-963b-f60fece0df2d",
"data-gatherer": "array-data",
"timestamp": "2024-06-01T12:00:00Z",
"data": [],
"schema_version": "v1"
}`,
expectError: `failed to parse DataReading.Data for gatherer "array-data": unknown type`,
},
{
name: "Incorrect timestamp format",
input: `{
"cluster_id": "d58f298d-b8c1-4d99-aa85-c27d9aec6f97",
"data-gatherer": "bad-timestamp",
"timestamp": "not-a-timestamp",
"data": {
"items": []
},
"schema_version": "v1"
}`,
expectError: `failed to parse DataReading: parsing time "not-a-timestamp" as "2006-01-02T15:04:05Z07:00": cannot parse "not-a-timestamp" as "2006"`,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
var dr DataReading
err := dr.UnmarshalJSON([]byte(tt.input))
if tt.expectError != "" {
assert.EqualError(t, err, tt.expectError)
return
}
assert.NoError(t, err)
assert.IsType(t, tt.wantDataType, dr.Data)
})
}
}
8 changes: 6 additions & 2 deletions pkg/echo/echo_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import (
"testing"
"time"

"k8s.io/apimachinery/pkg/version"

"github.com/jetstack/preflight/api"
)

Expand All @@ -34,8 +36,10 @@ func TestEchoServerRequestResponse(t *testing.T) {
ClusterID: "test_suite_cluster",
DataGatherer: "dummy",
Timestamp: api.Time{Time: time.Now()},
Data: map[string]string{
"test": "test",
Data: &api.DiscoveryData{
ServerVersion: &version.Info{
GitVersion: "v1.20.0",
},
},
SchemaVersion: "2.0.0",
},
Expand Down