Skip to content

Commit a199aaf

Browse files
rashmigottipatirashmigottipati
committed
refactor parsing logic to be feature gated
Signed-off-by: Rashmi Gottipati <rgottipa@redhat.com>
1 parent 477accb commit a199aaf

3 files changed

Lines changed: 110 additions & 41 deletions

File tree

‎internal/operator-controller/bundleutil/bundle.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111

1212
ocv1 "github.com/operator-framework/operator-controller/api/v1"
1313
"github.com/operator-framework/operator-controller/internal/operator-controller/bundle"
14+
"github.com/operator-framework/operator-controller/internal/operator-controller/features"
1415
)
1516

1617
func GetVersionAndRelease(b declcfg.Bundle) (*bundle.VersionRelease, error) {
@@ -28,8 +29,8 @@ func parseVersionRelease(pkgData json.RawMessage) (*bundle.VersionRelease, error
2829
return nil, fmt.Errorf("error unmarshalling package property: %w", err)
2930
}
3031

31-
// If the bundle has an explicit release field, use it
32-
if pkg.Release != "" {
32+
// When CompositeVersionComparison is enabled and bundle has explicit release field, use it
33+
if features.OperatorControllerFeatureGate.Enabled(features.CompositeVersionComparison) && pkg.Release != "" {
3334
vers, err := bsemver.Parse(pkg.Version)
3435
if err != nil {
3536
return nil, fmt.Errorf("error parsing version %q: %w", pkg.Version, err)

‎internal/operator-controller/bundleutil/bundle_test.go‎

Lines changed: 101 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212

1313
"github.com/operator-framework/operator-controller/internal/operator-controller/bundle"
1414
"github.com/operator-framework/operator-controller/internal/operator-controller/bundleutil"
15+
"github.com/operator-framework/operator-controller/internal/operator-controller/features"
1516
)
1617

1718
func TestGetVersionAndRelease(t *testing.T) {
@@ -69,44 +70,6 @@ func TestGetVersionAndRelease(t *testing.T) {
6970
pkgProperty: nil,
7071
wantErr: true,
7172
},
72-
{
73-
name: "explicit release field - takes precedence over build metadata",
74-
pkgProperty: &property.Property{
75-
Type: property.TypePackage,
76-
Value: json.RawMessage(`{"version": "1.0.0+ignored", "release": "2"}`),
77-
},
78-
wantVersionRelease: &bundle.VersionRelease{
79-
Version: bsemver.MustParse("1.0.0+ignored"),
80-
Release: bundle.Release([]bsemver.PRVersion{
81-
{VersionNum: 2, IsNum: true},
82-
}),
83-
},
84-
wantErr: false,
85-
},
86-
{
87-
name: "explicit release field - complex release",
88-
pkgProperty: &property.Property{
89-
Type: property.TypePackage,
90-
Value: json.RawMessage(`{"version": "2.1.0", "release": "1.alpha.3"}`),
91-
},
92-
wantVersionRelease: &bundle.VersionRelease{
93-
Version: bsemver.MustParse("2.1.0"),
94-
Release: bundle.Release([]bsemver.PRVersion{
95-
{VersionNum: 1, IsNum: true},
96-
{VersionStr: "alpha"},
97-
{VersionNum: 3, IsNum: true},
98-
}),
99-
},
100-
wantErr: false,
101-
},
102-
{
103-
name: "explicit release field - invalid release",
104-
pkgProperty: &property.Property{
105-
Type: property.TypePackage,
106-
Value: json.RawMessage(`{"version": "1.0.0", "release": "001"}`),
107-
},
108-
wantErr: true,
109-
},
11073
}
11174

11275
for _, tc := range tests {
@@ -131,3 +94,103 @@ func TestGetVersionAndRelease(t *testing.T) {
13194
})
13295
}
13396
}
97+
98+
// TestGetVersionAndRelease_WithCompositeVersionComparison tests the feature-gated parsing behavior.
99+
// This test detects the current feature gate state and validates the correct behavior path.
100+
// When CompositeVersionComparison is enabled (experimental deployments), bundles with explicit
101+
// pkg.Release field use that value. When disabled (standard deployments), explicit release is
102+
// ignored and parsing falls back to legacy build metadata.
103+
func TestGetVersionAndRelease_WithCompositeVersionComparison(t *testing.T) {
104+
// Detect current feature gate state and test the appropriate path
105+
if features.OperatorControllerFeatureGate.Enabled(features.CompositeVersionComparison) {
106+
t.Log("CompositeVersionComparison enabled - testing explicit release field parsing")
107+
108+
tests := []struct {
109+
name string
110+
pkgProperty *property.Property
111+
wantVersionRelease *bundle.VersionRelease
112+
wantErr bool
113+
}{
114+
{
115+
name: "explicit release field - takes precedence over build metadata",
116+
pkgProperty: &property.Property{
117+
Type: property.TypePackage,
118+
Value: json.RawMessage(`{"version": "1.0.0+ignored", "release": "2"}`),
119+
},
120+
wantVersionRelease: &bundle.VersionRelease{
121+
Version: bsemver.MustParse("1.0.0+ignored"),
122+
Release: bundle.Release([]bsemver.PRVersion{
123+
{VersionNum: 2, IsNum: true},
124+
}),
125+
},
126+
wantErr: false,
127+
},
128+
{
129+
name: "explicit release field - complex release",
130+
pkgProperty: &property.Property{
131+
Type: property.TypePackage,
132+
Value: json.RawMessage(`{"version": "2.1.0", "release": "1.alpha.3"}`),
133+
},
134+
wantVersionRelease: &bundle.VersionRelease{
135+
Version: bsemver.MustParse("2.1.0"),
136+
Release: bundle.Release([]bsemver.PRVersion{
137+
{VersionNum: 1, IsNum: true},
138+
{VersionStr: "alpha"},
139+
{VersionNum: 3, IsNum: true},
140+
}),
141+
},
142+
wantErr: false,
143+
},
144+
{
145+
name: "explicit release field - invalid release",
146+
pkgProperty: &property.Property{
147+
Type: property.TypePackage,
148+
Value: json.RawMessage(`{"version": "1.0.0", "release": "001"}`),
149+
},
150+
wantErr: true,
151+
},
152+
}
153+
154+
for _, tc := range tests {
155+
t.Run(tc.name, func(t *testing.T) {
156+
bundle := declcfg.Bundle{
157+
Name: "test-bundle",
158+
Properties: []property.Property{*tc.pkgProperty},
159+
}
160+
161+
actual, err := bundleutil.GetVersionAndRelease(bundle)
162+
if tc.wantErr {
163+
require.Error(t, err)
164+
} else {
165+
require.NoError(t, err)
166+
require.Equal(t, tc.wantVersionRelease, actual)
167+
}
168+
})
169+
}
170+
} else {
171+
t.Log("CompositeVersionComparison disabled - explicit release field ignored, falls back to build metadata")
172+
173+
// When gate disabled, explicit release field is ignored and parsing falls back to legacy behavior
174+
bundleWithExplicitRelease := declcfg.Bundle{
175+
Name: "test-bundle",
176+
Properties: []property.Property{
177+
{
178+
Type: property.TypePackage,
179+
Value: json.RawMessage(`{"version": "1.0.0+2", "release": "999"}`),
180+
},
181+
},
182+
}
183+
184+
actual, err := bundleutil.GetVersionAndRelease(bundleWithExplicitRelease)
185+
require.NoError(t, err)
186+
187+
// Should parse build metadata (+2), not explicit release field (999)
188+
expected := &bundle.VersionRelease{
189+
Version: bsemver.MustParse("1.0.0"),
190+
Release: bundle.Release([]bsemver.PRVersion{
191+
{VersionNum: 2, IsNum: true},
192+
}),
193+
}
194+
require.Equal(t, expected, actual)
195+
}
196+
}

‎internal/operator-controller/catalogmetadata/filter/bundle_predicates_test.go‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"github.com/operator-framework/operator-controller/internal/operator-controller/bundle"
1414
"github.com/operator-framework/operator-controller/internal/operator-controller/catalogmetadata/compare"
1515
"github.com/operator-framework/operator-controller/internal/operator-controller/catalogmetadata/filter"
16+
"github.com/operator-framework/operator-controller/internal/operator-controller/features"
1617
)
1718

1819
func TestInSemverRange(t *testing.T) {
@@ -171,8 +172,12 @@ func TestSameVersionHigherRelease(t *testing.T) {
171172
assert.False(t, f(testBundle))
172173
})
173174

174-
// Test with explicit pkg.Release field (new bundle format)
175+
// Test with explicit pkg.Release field (new bundle format) - only when feature gate enabled
175176
t.Run("explicit release field - higher release matches", func(t *testing.T) {
177+
if !features.OperatorControllerFeatureGate.Enabled(features.CompositeVersionComparison) {
178+
t.Skip("CompositeVersionComparison disabled - explicit release field not parsed")
179+
}
180+
176181
expect, err := bundle.NewLegacyRegistryV1VersionRelease("2.0.0+1")
177182
require.NoError(t, err)
178183

0 commit comments

Comments
 (0)