From f58fc81439bb0d65ac75e355b4e57229f26e25fa Mon Sep 17 00:00:00 2001 From: Chris Reed Date: Tue, 7 Apr 2026 10:24:40 -0500 Subject: [PATCH 1/5] feat(core): Add v1 migration schemas. --- migrations/artifact/artifact.go | 57 +++++ migrations/artifact/artifact_test.go | 70 +++++ migrations/artifact/metadata/metadata.go | 50 ++++ migrations/artifact/v1/schema.go | 310 +++++++++++++++++++++++ migrations/artifact/v1/schema_test.go | 139 ++++++++++ 5 files changed, 626 insertions(+) create mode 100644 migrations/artifact/artifact.go create mode 100644 migrations/artifact/artifact_test.go create mode 100644 migrations/artifact/metadata/metadata.go create mode 100644 migrations/artifact/v1/schema.go create mode 100644 migrations/artifact/v1/schema_test.go diff --git a/migrations/artifact/artifact.go b/migrations/artifact/artifact.go new file mode 100644 index 00000000..380f0b09 --- /dev/null +++ b/migrations/artifact/artifact.go @@ -0,0 +1,57 @@ +package artifact + +import ( + "errors" + "fmt" + "io" + + "github.com/Masterminds/semver/v3" + metadata "github.com/opentdf/otdfctl/migrations/artifact/metadata" + artifactv1 "github.com/opentdf/otdfctl/migrations/artifact/v1" +) + +const CurrentSchemaVersion = artifactv1.SchemaVersion + +var ( + currentSchemaVersion = semver.MustParse(CurrentSchemaVersion) + + ErrInvalidSchemaVersion = errors.New("invalid artifact schema version") + ErrUnsupportedSchemaVersion = errors.New("unsupported artifact schema version") + ErrNotImplemented = errors.New("not implemented") +) + +type ArtifactOpts struct { + Version *semver.Version + Writer io.Writer +} + +type Artifact interface { + Build() error + Commit() error + Metadata() metadata.ArtifactMetadata + Summary() ([]byte, error) + Write() error +} + +func New(opts ArtifactOpts) (Artifact, error) { + version := opts.Version + if version == nil { + version = currentSchemaVersion + } + + doc, err := newDocumentForVersion(version, opts.Writer) + if err != nil { + return nil, err + } + + return doc, nil +} + +func newDocumentForVersion(version *semver.Version, writer io.Writer) (Artifact, error) { + switch version.Major() { + case 1: + return artifactv1.New(writer), nil + default: + return nil, fmt.Errorf("%w: %s", ErrUnsupportedSchemaVersion, version.Original()) + } +} diff --git a/migrations/artifact/artifact_test.go b/migrations/artifact/artifact_test.go new file mode 100644 index 00000000..c33844a3 --- /dev/null +++ b/migrations/artifact/artifact_test.go @@ -0,0 +1,70 @@ +package artifact_test + +import ( + "bytes" + "testing" + + "github.com/Masterminds/semver/v3" + "github.com/opentdf/otdfctl/migrations/artifact" + artifactmetadata "github.com/opentdf/otdfctl/migrations/artifact/metadata" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestNewRejectsUnsupportedSchemaVersion(t *testing.T) { + t.Parallel() + + _, err := artifact.New(artifact.ArtifactOpts{ + Version: semver.MustParse("v2.0.0"), + }) + require.Error(t, err) + assert.ErrorIs(t, err, artifact.ErrUnsupportedSchemaVersion) +} + +func TestNewDefaultsCurrentVersion(t *testing.T) { + t.Parallel() + + var buf bytes.Buffer + doc, err := artifact.New(artifact.ArtifactOpts{Writer: &buf}) + require.NoError(t, err) + + require.NoError(t, doc.Write()) + assert.Contains(t, buf.String(), `"schema": "v1.0.0"`) + assert.Contains(t, buf.String(), `"name": "`+artifactmetadata.ArtifactName+`"`) +} + +func TestArtifactSummaryReturnsEncodedJSON(t *testing.T) { + t.Parallel() + + doc, err := artifact.New(artifact.ArtifactOpts{}) + require.NoError(t, err) + + summary, err := doc.Summary() + require.NoError(t, err) + assert.JSONEq(t, `{ + "counts": { + "namespaces": 0, + "actions": 0, + "subject_condition_sets": 0, + "subject_mappings": 0, + "registered_resources": 0, + "obligation_triggers": 0, + "skipped": 0 + } + }`, string(summary)) +} + +func TestArtifactBuildAndCommitAreNotImplemented(t *testing.T) { + t.Parallel() + + doc, err := artifact.New(artifact.ArtifactOpts{}) + require.NoError(t, err) + + buildErr := doc.Build() + require.Error(t, buildErr) + assert.ErrorContains(t, buildErr, "not implemented") + + commitErr := doc.Commit() + require.Error(t, commitErr) + assert.ErrorContains(t, commitErr, "not implemented") +} diff --git a/migrations/artifact/metadata/metadata.go b/migrations/artifact/metadata/metadata.go new file mode 100644 index 00000000..d92487aa --- /dev/null +++ b/migrations/artifact/metadata/metadata.go @@ -0,0 +1,50 @@ +package metadata + +import ( + "time" + + "github.com/Masterminds/semver/v3" +) + +const ArtifactName = "policy-migration" + +type ArtifactMetadata struct { + SchemaValue string `json:"schema"` + NameValue string `json:"name"` + RunIDValue string `json:"run_id"` + CreatedAtValue time.Time `json:"created_at"` +} + +func New(schema, runID string, createdAt time.Time) ArtifactMetadata { + return ArtifactMetadata{ + SchemaValue: schema, + NameValue: ArtifactName, + RunIDValue: runID, + CreatedAtValue: createdAt, + } +} + +func (m ArtifactMetadata) Schema() *semver.Version { + if m.SchemaValue == "" { + return nil + } + + version, err := semver.NewVersion(m.SchemaValue) + if err != nil { + return nil + } + + return version +} + +func (m ArtifactMetadata) Name() string { + return m.NameValue +} + +func (m ArtifactMetadata) RunID() string { + return m.RunIDValue +} + +func (m ArtifactMetadata) CreatedAt() time.Time { + return m.CreatedAtValue +} diff --git a/migrations/artifact/v1/schema.go b/migrations/artifact/v1/schema.go new file mode 100644 index 00000000..3ce755ec --- /dev/null +++ b/migrations/artifact/v1/schema.go @@ -0,0 +1,310 @@ +package v1 + +import ( + "encoding/json" + "errors" + "fmt" + "io" + "time" + + "github.com/google/uuid" + artifactmetadata "github.com/opentdf/otdfctl/migrations/artifact/metadata" +) + +const SchemaVersion = "v1.0.0" + +var ( + ErrNilArtifact = errors.New("nil artifact") + ErrNilWriter = errors.New("nil writer") + ErrWriteArtifact = errors.New("write artifact") + ErrSummaryArtifact = errors.New("summary artifact") +) + +type artifact struct { + MetadataData artifactmetadata.ArtifactMetadata `json:"metadata"` + SummaryData Summary `json:"summary"` + Skipped []skippedEntry `json:"skipped"` + Namespaces []namespaceIndexEntry `json:"namespaces"` + Actions []actionRecord `json:"actions"` + SubjectConditionSets []subjectConditionSetRecord `json:"subject_condition_sets"` + SubjectMappings []subjectMappingRecord `json:"subject_mappings"` + RegisteredResources []registeredResourceRecord `json:"registered_resources"` + ObligationTriggers []obligationTriggerRecord `json:"obligation_triggers"` + writer io.Writer `json:"-"` +} + +type Summary struct { + Counts SummaryCounts `json:"counts"` +} + +type SummaryCounts struct { + Namespaces int `json:"namespaces"` + Actions int `json:"actions"` + SubjectConditionSets int `json:"subject_condition_sets"` + SubjectMappings int `json:"subject_mappings"` + RegisteredResources int `json:"registered_resources"` + ObligationTriggers int `json:"obligation_triggers"` + Skipped int `json:"skipped"` +} + +type skippedEntry struct { + Type string `json:"type"` + SkippedReasonCode string `json:"skipped_reason_code"` + SkippedReason string `json:"skipped_reason"` + Source skippedSource `json:"source"` + Context skippedContext `json:"context"` +} + +type skippedSource struct { + RegisteredResourceID string `json:"registered_resource_id,omitempty"` + RegisteredResourceValueID string `json:"registered_resource_value_id,omitempty"` + ActionID string `json:"action_id,omitempty"` + AttributeValueID string `json:"attribute_value_id,omitempty"` +} + +type skippedContext struct { + TargetNamespaceID string `json:"target_namespace_id,omitempty"` + TargetNamespaceFQN string `json:"target_namespace_fqn,omitempty"` +} + +type namespaceIndexEntry struct { + FQN string `json:"fqn"` + ID string `json:"id"` + Actions []string `json:"actions"` + SubjectConditionSets []string `json:"subject_condition_sets"` + SubjectMappings []string `json:"subject_mappings"` + RegisteredResources []string `json:"registered_resources"` + ObligationTriggers []string `json:"obligation_triggers"` +} + +type actionRecord struct { + Source actionSource `json:"source"` + Targets []actionTarget `json:"targets"` +} + +type actionSource struct { + ID string `json:"id"` + Name string `json:"name"` + NamespaceID *string `json:"namespace_id"` + IsStandard bool `json:"is_standard"` +} + +type actionTarget struct { + NamespaceID string `json:"namespace_id"` + NamespaceFQN string `json:"namespace_fqn"` + ID string `json:"id"` +} + +type subjectConditionSetRecord struct { + Source subjectConditionSetSource `json:"source"` + Targets []subjectConditionSetTarget `json:"targets"` +} + +type subjectConditionSetSource struct { + ID string `json:"id"` + Name string `json:"name"` + NamespaceID *string `json:"namespace_id"` +} + +type subjectConditionSetTarget struct { + NamespaceID string `json:"namespace_id"` + NamespaceFQN string `json:"namespace_fqn"` + ID string `json:"id"` +} + +type subjectMappingRecord struct { + Source subjectMappingSource `json:"source"` + Targets []subjectMappingTarget `json:"targets"` +} + +type subjectMappingSource struct { + ID string `json:"id"` + ActionIDs []string `json:"action_ids"` + SubjectConditionSetID string `json:"subject_condition_set_id"` + NamespaceID *string `json:"namespace_id"` + AttributeValueID string `json:"attribute_value_id"` +} + +type subjectMappingTarget struct { + NamespaceID string `json:"namespace_id"` + NamespaceFQN string `json:"namespace_fqn"` + ID string `json:"id"` + ActionIDs []string `json:"action_ids"` + SubjectConditionSetID string `json:"subject_condition_set_id"` + AttributeValueID string `json:"attribute_value_id"` +} + +type registeredResourceRecord struct { + Source registeredResourceSource `json:"source"` + Targets []registeredResourceTarget `json:"targets"` +} + +type registeredResourceSource struct { + ID string `json:"id"` + Name string `json:"name"` + NamespaceID *string `json:"namespace_id"` + Values []registeredResourceValue `json:"values"` +} + +type registeredResourceTarget struct { + NamespaceID string `json:"namespace_id"` + NamespaceFQN string `json:"namespace_fqn"` + ID string `json:"id"` + Values []registeredResourceValue `json:"values"` +} + +type registeredResourceValue struct { + ID string `json:"id"` + Value string `json:"value"` + ActionAttributeValues []actionAttributeValue `json:"action_attribute_values"` +} + +type actionAttributeValue struct { + ActionID string `json:"action_id"` + AttributeValueID string `json:"attribute_value_id"` +} + +type obligationTriggerRecord struct { + Source obligationTriggerSource `json:"source"` + Targets []obligationTriggerTarget `json:"targets"` +} + +type obligationTriggerSource struct { + ID string `json:"id"` + NamespaceID string `json:"namespace_id"` + NamespaceFQN string `json:"namespace_fqn"` + ActionID string `json:"action_id"` + ObligationValueID string `json:"obligation_value_id"` + AttributeValueID string `json:"attribute_value_id"` + ClientID string `json:"client_id"` +} + +type obligationTriggerTarget struct { + NamespaceID string `json:"namespace_id"` + NamespaceFQN string `json:"namespace_fqn"` + ActionID string `json:"action_id"` + ObligationValueID string `json:"obligation_value_id"` + AttributeValueID string `json:"attribute_value_id"` + ClientID string `json:"client_id"` + ID string `json:"id"` +} + +func New(writer io.Writer) *artifact { + doc := &artifact{ + MetadataData: artifactmetadata.New(SchemaVersion, uuid.NewString(), time.Now().UTC()), + Skipped: []skippedEntry{}, + Namespaces: []namespaceIndexEntry{}, + Actions: []actionRecord{}, + SubjectConditionSets: []subjectConditionSetRecord{}, + SubjectMappings: []subjectMappingRecord{}, + RegisteredResources: []registeredResourceRecord{}, + ObligationTriggers: []obligationTriggerRecord{}, + writer: writer, + } + _, _ = doc.Summary() + return doc +} + +func (a *artifact) Build() error { + return fmt.Errorf("artifact build for schema %s: not implemented", SchemaVersion) +} + +func (a *artifact) Commit() error { + return fmt.Errorf("artifact commit for schema %s: not implemented", SchemaVersion) +} + +func (a *artifact) Metadata() artifactmetadata.ArtifactMetadata { + if a == nil { + return artifactmetadata.ArtifactMetadata{} + } + return a.MetadataData +} + +func (a *artifact) Summary() ([]byte, error) { + if a == nil { + return nil, ErrNilArtifact + } + + a.SummaryData.Counts = SummaryCounts{ + Namespaces: len(a.Namespaces), + Actions: len(a.Actions), + SubjectConditionSets: len(a.SubjectConditionSets), + SubjectMappings: len(a.SubjectMappings), + RegisteredResources: len(a.RegisteredResources), + ObligationTriggers: len(a.ObligationTriggers), + Skipped: len(a.Skipped), + } + + encoded, err := json.Marshal(a.SummaryData) + if err != nil { + return nil, fmt.Errorf("%w: %w", ErrSummaryArtifact, err) + } + + return encoded, nil +} + +func (a *artifact) Write() error { + if a == nil { + return ErrNilArtifact + } + if a.writer == nil { + return ErrNilWriter + } + + a.normalize() + if _, err := a.Summary(); err != nil { + return err + } + + encoder := json.NewEncoder(a.writer) + encoder.SetIndent("", " ") + if err := encoder.Encode(a); err != nil { + return fmt.Errorf("%w: %w", ErrWriteArtifact, err) + } + + return nil +} + +func (a *artifact) normalize() { + if a == nil { + return + } + + schema := SchemaVersion + if version := a.MetadataData.Schema(); version != nil { + schema = version.Original() + } + + runID := a.MetadataData.RunID() + if runID == "" { + runID = uuid.NewString() + } + + createdAt := a.MetadataData.CreatedAt() + if createdAt.IsZero() { + createdAt = time.Now().UTC() + } + + a.MetadataData = artifactmetadata.New(schema, runID, createdAt) + if a.Skipped == nil { + a.Skipped = []skippedEntry{} + } + if a.Namespaces == nil { + a.Namespaces = []namespaceIndexEntry{} + } + if a.Actions == nil { + a.Actions = []actionRecord{} + } + if a.SubjectConditionSets == nil { + a.SubjectConditionSets = []subjectConditionSetRecord{} + } + if a.SubjectMappings == nil { + a.SubjectMappings = []subjectMappingRecord{} + } + if a.RegisteredResources == nil { + a.RegisteredResources = []registeredResourceRecord{} + } + if a.ObligationTriggers == nil { + a.ObligationTriggers = []obligationTriggerRecord{} + } +} diff --git a/migrations/artifact/v1/schema_test.go b/migrations/artifact/v1/schema_test.go new file mode 100644 index 00000000..46679849 --- /dev/null +++ b/migrations/artifact/v1/schema_test.go @@ -0,0 +1,139 @@ +package v1 + +import ( + "bytes" + "encoding/json" + "testing" + "time" + + artifactmetadata "github.com/opentdf/otdfctl/migrations/artifact/metadata" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestNewInitializesCanonicalShape(t *testing.T) { + t.Parallel() + + doc := New(nil) + + require.NotNil(t, doc) + assert.Equal(t, SchemaVersion, doc.MetadataData.SchemaValue) + assert.Equal(t, artifactmetadata.ArtifactName, doc.MetadataData.Name()) + assert.NotEmpty(t, doc.MetadataData.RunID()) + assert.WithinDuration(t, time.Now().UTC(), doc.MetadataData.CreatedAt(), time.Minute) + assert.Empty(t, doc.Actions) + assert.Empty(t, doc.Skipped) + + summaryBytes, err := doc.Summary() + require.NoError(t, err) + + var summary Summary + require.NoError(t, json.Unmarshal(summaryBytes, &summary)) + assert.Equal(t, 0, summary.Counts.Actions) + assert.Equal(t, 0, summary.Counts.Skipped) +} + +func TestSummaryReturnsEncodedJSON(t *testing.T) { + t.Parallel() + + doc := New(nil) + doc.Actions = append(doc.Actions, actionRecord{}) + doc.Skipped = append(doc.Skipped, skippedEntry{}) + + summaryBytes, err := doc.Summary() + require.NoError(t, err) + + var summary Summary + require.NoError(t, json.Unmarshal(summaryBytes, &summary)) + assert.Equal(t, SummaryCounts{ + Namespaces: 0, + Actions: 1, + SubjectConditionSets: 0, + SubjectMappings: 0, + RegisteredResources: 0, + ObligationTriggers: 0, + Skipped: 1, + }, summary.Counts) +} + +func TestWriteProducesJSONDocument(t *testing.T) { + t.Parallel() + + var buf bytes.Buffer + doc := New(&buf) + doc.Actions = append(doc.Actions, actionRecord{ + Source: actionSource{ + ID: "action-export-legacy", + Name: "export", + IsStandard: false, + }, + Targets: []actionTarget{ + { + NamespaceID: "ns-finance-001", + NamespaceFQN: "https://finance.example.com", + ID: "action-export-finance", + }, + }, + }) + doc.Skipped = append(doc.Skipped, skippedEntry{ + Type: "registered_resource_value_action_attribute_value", + SkippedReasonCode: "ambiguous_target_action", + SkippedReason: "Could not determine a safe target action for this RAAV.", + }) + + require.NoError(t, doc.Write()) + + var decoded artifact + require.NoError(t, json.Unmarshal(buf.Bytes(), &decoded)) + + assert.Equal(t, SchemaVersion, decoded.MetadataData.SchemaValue) + assert.Equal(t, artifactmetadata.ArtifactName, decoded.MetadataData.Name()) + assert.NotEmpty(t, decoded.MetadataData.RunID()) + assert.NotEmpty(t, decoded.MetadataData.CreatedAt()) + assert.Equal(t, 1, decoded.SummaryData.Counts.Actions) + assert.Equal(t, 1, decoded.SummaryData.Counts.Skipped) +} + +func TestWriteFailsWithoutWriter(t *testing.T) { + t.Parallel() + + doc := New(nil) + + err := doc.Write() + require.Error(t, err) + assert.ErrorIs(t, err, ErrNilWriter) +} + +func TestWriteFailsWithNilArtifact(t *testing.T) { + t.Parallel() + + var doc *artifact + + err := doc.Write() + require.Error(t, err) + assert.ErrorIs(t, err, ErrNilArtifact) +} + +func TestSummaryFailsWithNilArtifact(t *testing.T) { + t.Parallel() + + var doc *artifact + + _, err := doc.Summary() + require.Error(t, err) + assert.ErrorIs(t, err, ErrNilArtifact) +} + +func TestNormalizeFillsMissingMetadata(t *testing.T) { + t.Parallel() + + doc := &artifact{} + doc.normalize() + + assert.Equal(t, SchemaVersion, doc.MetadataData.SchemaValue) + assert.Equal(t, artifactmetadata.ArtifactName, doc.MetadataData.Name()) + assert.NotEmpty(t, doc.MetadataData.RunID()) + assert.False(t, doc.MetadataData.CreatedAt().IsZero()) + require.NotNil(t, doc.Actions) + require.NotNil(t, doc.Skipped) +} From 5bb47b9bb7999301176e75ad50e453d7df8cd5b3 Mon Sep 17 00:00:00 2001 From: Chris Reed Date: Tue, 7 Apr 2026 10:28:31 -0500 Subject: [PATCH 2/5] aggregate test with correct package. --- migrations/artifact/artifact_test.go | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/migrations/artifact/artifact_test.go b/migrations/artifact/artifact_test.go index c33844a3..9ef7c5bb 100644 --- a/migrations/artifact/artifact_test.go +++ b/migrations/artifact/artifact_test.go @@ -1,11 +1,10 @@ -package artifact_test +package artifact import ( "bytes" "testing" "github.com/Masterminds/semver/v3" - "github.com/opentdf/otdfctl/migrations/artifact" artifactmetadata "github.com/opentdf/otdfctl/migrations/artifact/metadata" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -14,18 +13,18 @@ import ( func TestNewRejectsUnsupportedSchemaVersion(t *testing.T) { t.Parallel() - _, err := artifact.New(artifact.ArtifactOpts{ + _, err := New(ArtifactOpts{ Version: semver.MustParse("v2.0.0"), }) require.Error(t, err) - assert.ErrorIs(t, err, artifact.ErrUnsupportedSchemaVersion) + assert.ErrorIs(t, err, ErrUnsupportedSchemaVersion) } func TestNewDefaultsCurrentVersion(t *testing.T) { t.Parallel() var buf bytes.Buffer - doc, err := artifact.New(artifact.ArtifactOpts{Writer: &buf}) + doc, err := New(ArtifactOpts{Writer: &buf}) require.NoError(t, err) require.NoError(t, doc.Write()) @@ -36,7 +35,7 @@ func TestNewDefaultsCurrentVersion(t *testing.T) { func TestArtifactSummaryReturnsEncodedJSON(t *testing.T) { t.Parallel() - doc, err := artifact.New(artifact.ArtifactOpts{}) + doc, err := New(ArtifactOpts{}) require.NoError(t, err) summary, err := doc.Summary() @@ -57,7 +56,7 @@ func TestArtifactSummaryReturnsEncodedJSON(t *testing.T) { func TestArtifactBuildAndCommitAreNotImplemented(t *testing.T) { t.Parallel() - doc, err := artifact.New(artifact.ArtifactOpts{}) + doc, err := New(ArtifactOpts{}) require.NoError(t, err) buildErr := doc.Build() From f0efd7a909b014036d518ab7bce51f49b9a28ae0 Mon Sep 17 00:00:00 2001 From: Chris Reed Date: Tue, 7 Apr 2026 10:58:29 -0500 Subject: [PATCH 3/5] fix code comments. --- migrations/artifact/v1/schema.go | 92 +++++++-------------------- migrations/artifact/v1/schema_test.go | 34 ---------- 2 files changed, 24 insertions(+), 102 deletions(-) diff --git a/migrations/artifact/v1/schema.go b/migrations/artifact/v1/schema.go index 3ce755ec..38e63894 100644 --- a/migrations/artifact/v1/schema.go +++ b/migrations/artifact/v1/schema.go @@ -14,7 +14,6 @@ import ( const SchemaVersion = "v1.0.0" var ( - ErrNilArtifact = errors.New("nil artifact") ErrNilWriter = errors.New("nil writer") ErrWriteArtifact = errors.New("write artifact") ErrSummaryArtifact = errors.New("summary artifact") @@ -190,7 +189,7 @@ type obligationTriggerTarget struct { } func New(writer io.Writer) *artifact { - doc := &artifact{ + return &artifact{ MetadataData: artifactmetadata.New(SchemaVersion, uuid.NewString(), time.Now().UTC()), Skipped: []skippedEntry{}, Namespaces: []namespaceIndexEntry{}, @@ -201,8 +200,6 @@ func New(writer io.Writer) *artifact { ObligationTriggers: []obligationTriggerRecord{}, writer: writer, } - _, _ = doc.Summary() - return doc } func (a *artifact) Build() error { @@ -214,28 +211,23 @@ func (a *artifact) Commit() error { } func (a *artifact) Metadata() artifactmetadata.ArtifactMetadata { - if a == nil { - return artifactmetadata.ArtifactMetadata{} - } return a.MetadataData } func (a *artifact) Summary() ([]byte, error) { - if a == nil { - return nil, ErrNilArtifact - } - - a.SummaryData.Counts = SummaryCounts{ - Namespaces: len(a.Namespaces), - Actions: len(a.Actions), - SubjectConditionSets: len(a.SubjectConditionSets), - SubjectMappings: len(a.SubjectMappings), - RegisteredResources: len(a.RegisteredResources), - ObligationTriggers: len(a.ObligationTriggers), - Skipped: len(a.Skipped), + summary := Summary{ + Counts: SummaryCounts{ + Namespaces: len(a.Namespaces), + Actions: len(a.Actions), + SubjectConditionSets: len(a.SubjectConditionSets), + SubjectMappings: len(a.SubjectMappings), + RegisteredResources: len(a.RegisteredResources), + ObligationTriggers: len(a.ObligationTriggers), + Skipped: len(a.Skipped), + }, } - encoded, err := json.Marshal(a.SummaryData) + encoded, err := json.Marshal(summary) if err != nil { return nil, fmt.Errorf("%w: %w", ErrSummaryArtifact, err) } @@ -244,17 +236,11 @@ func (a *artifact) Summary() ([]byte, error) { } func (a *artifact) Write() error { - if a == nil { - return ErrNilArtifact - } if a.writer == nil { return ErrNilWriter } - a.normalize() - if _, err := a.Summary(); err != nil { - return err - } + a.updateSummary() encoder := json.NewEncoder(a.writer) encoder.SetIndent("", " ") @@ -265,46 +251,16 @@ func (a *artifact) Write() error { return nil } -func (a *artifact) normalize() { - if a == nil { - return - } - - schema := SchemaVersion - if version := a.MetadataData.Schema(); version != nil { - schema = version.Original() - } - - runID := a.MetadataData.RunID() - if runID == "" { - runID = uuid.NewString() - } - - createdAt := a.MetadataData.CreatedAt() - if createdAt.IsZero() { - createdAt = time.Now().UTC() - } - - a.MetadataData = artifactmetadata.New(schema, runID, createdAt) - if a.Skipped == nil { - a.Skipped = []skippedEntry{} - } - if a.Namespaces == nil { - a.Namespaces = []namespaceIndexEntry{} - } - if a.Actions == nil { - a.Actions = []actionRecord{} - } - if a.SubjectConditionSets == nil { - a.SubjectConditionSets = []subjectConditionSetRecord{} - } - if a.SubjectMappings == nil { - a.SubjectMappings = []subjectMappingRecord{} - } - if a.RegisteredResources == nil { - a.RegisteredResources = []registeredResourceRecord{} - } - if a.ObligationTriggers == nil { - a.ObligationTriggers = []obligationTriggerRecord{} +func (a *artifact) updateSummary() { + a.SummaryData = Summary{ + Counts: SummaryCounts{ + Namespaces: len(a.Namespaces), + Actions: len(a.Actions), + SubjectConditionSets: len(a.SubjectConditionSets), + SubjectMappings: len(a.SubjectMappings), + RegisteredResources: len(a.RegisteredResources), + ObligationTriggers: len(a.ObligationTriggers), + Skipped: len(a.Skipped), + }, } } diff --git a/migrations/artifact/v1/schema_test.go b/migrations/artifact/v1/schema_test.go index 46679849..9da6a365 100644 --- a/migrations/artifact/v1/schema_test.go +++ b/migrations/artifact/v1/schema_test.go @@ -103,37 +103,3 @@ func TestWriteFailsWithoutWriter(t *testing.T) { require.Error(t, err) assert.ErrorIs(t, err, ErrNilWriter) } - -func TestWriteFailsWithNilArtifact(t *testing.T) { - t.Parallel() - - var doc *artifact - - err := doc.Write() - require.Error(t, err) - assert.ErrorIs(t, err, ErrNilArtifact) -} - -func TestSummaryFailsWithNilArtifact(t *testing.T) { - t.Parallel() - - var doc *artifact - - _, err := doc.Summary() - require.Error(t, err) - assert.ErrorIs(t, err, ErrNilArtifact) -} - -func TestNormalizeFillsMissingMetadata(t *testing.T) { - t.Parallel() - - doc := &artifact{} - doc.normalize() - - assert.Equal(t, SchemaVersion, doc.MetadataData.SchemaValue) - assert.Equal(t, artifactmetadata.ArtifactName, doc.MetadataData.Name()) - assert.NotEmpty(t, doc.MetadataData.RunID()) - assert.False(t, doc.MetadataData.CreatedAt().IsZero()) - require.NotNil(t, doc.Actions) - require.NotNil(t, doc.Skipped) -} From 32d8852b1d2a9d31b45a81c32aba5222566dd162 Mon Sep 17 00:00:00 2001 From: Chris Reed Date: Tue, 7 Apr 2026 11:06:25 -0500 Subject: [PATCH 4/5] fix code comments. --- migrations/artifact/artifact.go | 2 +- migrations/artifact/artifact_test.go | 19 +++++++++++++++---- migrations/artifact/v1/schema.go | 17 +++++++++-------- migrations/artifact/v1/schema_test.go | 17 ++++++++++------- 4 files changed, 35 insertions(+), 20 deletions(-) diff --git a/migrations/artifact/artifact.go b/migrations/artifact/artifact.go index 380f0b09..2064e25e 100644 --- a/migrations/artifact/artifact.go +++ b/migrations/artifact/artifact.go @@ -50,7 +50,7 @@ func New(opts ArtifactOpts) (Artifact, error) { func newDocumentForVersion(version *semver.Version, writer io.Writer) (Artifact, error) { switch version.Major() { case 1: - return artifactv1.New(writer), nil + return artifactv1.New(writer) default: return nil, fmt.Errorf("%w: %s", ErrUnsupportedSchemaVersion, version.Original()) } diff --git a/migrations/artifact/artifact_test.go b/migrations/artifact/artifact_test.go index 9ef7c5bb..ffe89b64 100644 --- a/migrations/artifact/artifact_test.go +++ b/migrations/artifact/artifact_test.go @@ -6,6 +6,7 @@ import ( "github.com/Masterminds/semver/v3" artifactmetadata "github.com/opentdf/otdfctl/migrations/artifact/metadata" + artifactv1 "github.com/opentdf/otdfctl/migrations/artifact/v1" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -20,6 +21,14 @@ func TestNewRejectsUnsupportedSchemaVersion(t *testing.T) { assert.ErrorIs(t, err, ErrUnsupportedSchemaVersion) } +func TestNewRejectsNilWriter(t *testing.T) { + t.Parallel() + + _, err := New(ArtifactOpts{}) + require.Error(t, err) + assert.ErrorIs(t, err, artifactv1.ErrNilWriter) +} + func TestNewDefaultsCurrentVersion(t *testing.T) { t.Parallel() @@ -35,7 +44,8 @@ func TestNewDefaultsCurrentVersion(t *testing.T) { func TestArtifactSummaryReturnsEncodedJSON(t *testing.T) { t.Parallel() - doc, err := New(ArtifactOpts{}) + var buf bytes.Buffer + doc, err := New(ArtifactOpts{Writer: &buf}) require.NoError(t, err) summary, err := doc.Summary() @@ -56,14 +66,15 @@ func TestArtifactSummaryReturnsEncodedJSON(t *testing.T) { func TestArtifactBuildAndCommitAreNotImplemented(t *testing.T) { t.Parallel() - doc, err := New(ArtifactOpts{}) + var buf bytes.Buffer + doc, err := New(ArtifactOpts{Writer: &buf}) require.NoError(t, err) buildErr := doc.Build() require.Error(t, buildErr) - assert.ErrorContains(t, buildErr, "not implemented") + assert.ErrorIs(t, buildErr, artifactv1.ErrNotImplemented) commitErr := doc.Commit() require.Error(t, commitErr) - assert.ErrorContains(t, commitErr, "not implemented") + assert.ErrorIs(t, commitErr, artifactv1.ErrNotImplemented) } diff --git a/migrations/artifact/v1/schema.go b/migrations/artifact/v1/schema.go index 38e63894..e67d48ca 100644 --- a/migrations/artifact/v1/schema.go +++ b/migrations/artifact/v1/schema.go @@ -14,6 +14,7 @@ import ( const SchemaVersion = "v1.0.0" var ( + ErrNotImplemented = errors.New("not implemented") ErrNilWriter = errors.New("nil writer") ErrWriteArtifact = errors.New("write artifact") ErrSummaryArtifact = errors.New("summary artifact") @@ -188,7 +189,11 @@ type obligationTriggerTarget struct { ID string `json:"id"` } -func New(writer io.Writer) *artifact { +func New(writer io.Writer) (*artifact, error) { + if writer == nil { + return nil, ErrNilWriter + } + return &artifact{ MetadataData: artifactmetadata.New(SchemaVersion, uuid.NewString(), time.Now().UTC()), Skipped: []skippedEntry{}, @@ -199,15 +204,15 @@ func New(writer io.Writer) *artifact { RegisteredResources: []registeredResourceRecord{}, ObligationTriggers: []obligationTriggerRecord{}, writer: writer, - } + }, nil } func (a *artifact) Build() error { - return fmt.Errorf("artifact build for schema %s: not implemented", SchemaVersion) + return fmt.Errorf("%w: artifact build for schema %s", ErrNotImplemented, SchemaVersion) } func (a *artifact) Commit() error { - return fmt.Errorf("artifact commit for schema %s: not implemented", SchemaVersion) + return fmt.Errorf("%w: artifact commit for schema %s", ErrNotImplemented, SchemaVersion) } func (a *artifact) Metadata() artifactmetadata.ArtifactMetadata { @@ -236,10 +241,6 @@ func (a *artifact) Summary() ([]byte, error) { } func (a *artifact) Write() error { - if a.writer == nil { - return ErrNilWriter - } - a.updateSummary() encoder := json.NewEncoder(a.writer) diff --git a/migrations/artifact/v1/schema_test.go b/migrations/artifact/v1/schema_test.go index 9da6a365..4ab584fb 100644 --- a/migrations/artifact/v1/schema_test.go +++ b/migrations/artifact/v1/schema_test.go @@ -14,7 +14,9 @@ import ( func TestNewInitializesCanonicalShape(t *testing.T) { t.Parallel() - doc := New(nil) + var buf bytes.Buffer + doc, err := New(&buf) + require.NoError(t, err) require.NotNil(t, doc) assert.Equal(t, SchemaVersion, doc.MetadataData.SchemaValue) @@ -36,7 +38,9 @@ func TestNewInitializesCanonicalShape(t *testing.T) { func TestSummaryReturnsEncodedJSON(t *testing.T) { t.Parallel() - doc := New(nil) + var buf bytes.Buffer + doc, err := New(&buf) + require.NoError(t, err) doc.Actions = append(doc.Actions, actionRecord{}) doc.Skipped = append(doc.Skipped, skippedEntry{}) @@ -60,7 +64,8 @@ func TestWriteProducesJSONDocument(t *testing.T) { t.Parallel() var buf bytes.Buffer - doc := New(&buf) + doc, err := New(&buf) + require.NoError(t, err) doc.Actions = append(doc.Actions, actionRecord{ Source: actionSource{ ID: "action-export-legacy", @@ -94,12 +99,10 @@ func TestWriteProducesJSONDocument(t *testing.T) { assert.Equal(t, 1, decoded.SummaryData.Counts.Skipped) } -func TestWriteFailsWithoutWriter(t *testing.T) { +func TestNewFailsWithoutWriter(t *testing.T) { t.Parallel() - doc := New(nil) - - err := doc.Write() + _, err := New(nil) require.Error(t, err) assert.ErrorIs(t, err, ErrNilWriter) } From eef4e892be96fd0bec9120804c9cc6626f4ed254 Mon Sep 17 00:00:00 2001 From: Chris Reed Date: Tue, 7 Apr 2026 11:20:51 -0500 Subject: [PATCH 5/5] fix linting. --- migrations/artifact/artifact_test.go | 12 ++++-------- migrations/artifact/v1/schema_test.go | 3 +-- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/migrations/artifact/artifact_test.go b/migrations/artifact/artifact_test.go index ffe89b64..23e17a75 100644 --- a/migrations/artifact/artifact_test.go +++ b/migrations/artifact/artifact_test.go @@ -17,16 +17,14 @@ func TestNewRejectsUnsupportedSchemaVersion(t *testing.T) { _, err := New(ArtifactOpts{ Version: semver.MustParse("v2.0.0"), }) - require.Error(t, err) - assert.ErrorIs(t, err, ErrUnsupportedSchemaVersion) + require.ErrorIs(t, err, ErrUnsupportedSchemaVersion) } func TestNewRejectsNilWriter(t *testing.T) { t.Parallel() _, err := New(ArtifactOpts{}) - require.Error(t, err) - assert.ErrorIs(t, err, artifactv1.ErrNilWriter) + require.ErrorIs(t, err, artifactv1.ErrNilWriter) } func TestNewDefaultsCurrentVersion(t *testing.T) { @@ -71,10 +69,8 @@ func TestArtifactBuildAndCommitAreNotImplemented(t *testing.T) { require.NoError(t, err) buildErr := doc.Build() - require.Error(t, buildErr) - assert.ErrorIs(t, buildErr, artifactv1.ErrNotImplemented) + require.ErrorIs(t, buildErr, artifactv1.ErrNotImplemented) commitErr := doc.Commit() - require.Error(t, commitErr) - assert.ErrorIs(t, commitErr, artifactv1.ErrNotImplemented) + require.ErrorIs(t, commitErr, artifactv1.ErrNotImplemented) } diff --git a/migrations/artifact/v1/schema_test.go b/migrations/artifact/v1/schema_test.go index 4ab584fb..91c579ba 100644 --- a/migrations/artifact/v1/schema_test.go +++ b/migrations/artifact/v1/schema_test.go @@ -103,6 +103,5 @@ func TestNewFailsWithoutWriter(t *testing.T) { t.Parallel() _, err := New(nil) - require.Error(t, err) - assert.ErrorIs(t, err, ErrNilWriter) + require.ErrorIs(t, err, ErrNilWriter) }