Skip to content

Commit f1067da

Browse files
joelanfordjoelanford
authored
Enable CatalogMetadataAPI via explicit flag, fix syncing issues (operator-framework#138)
1 parent 64a9fe5 commit f1067da

File tree

7 files changed

+124
-29
lines changed

7 files changed

+124
-29
lines changed

‎config/default/manager_auth_proxy_patch.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,5 +50,5 @@ spec:
5050
- "--health-probe-bind-address=:8081"
5151
- "--metrics-bind-address=127.0.0.1:8080"
5252
- "--leader-elect"
53-
- "--feature-gates=PackagesBundleMetadataAPIs=true"
53+
- "--feature-gates=PackagesBundleMetadataAPIs=true,CatalogMetadataAPI=true"
5454

‎go.mod

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ require (
99
github.com/onsi/gomega v1.27.7
1010
github.com/operator-framework/operator-registry v1.27.1
1111
github.com/spf13/pflag v1.0.5
12+
github.com/stretchr/testify v1.8.0
1213
k8s.io/api v0.26.1
1314
k8s.io/apimachinery v0.26.1
1415
k8s.io/client-go v0.26.1
@@ -55,6 +56,7 @@ require (
5556
github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect
5657
github.com/operator-framework/api v0.17.4-0.20230223191600-0131a6301e42 // indirect
5758
github.com/pkg/errors v0.9.1 // indirect
59+
github.com/pmezard/go-difflib v1.0.0 // indirect
5860
github.com/prometheus/client_golang v1.14.0 // indirect
5961
github.com/prometheus/client_model v0.3.0 // indirect
6062
github.com/prometheus/common v0.37.0 // indirect

‎go.sum

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -300,6 +300,7 @@ github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An
300300
github.com/stoewer/go-strcase v1.2.0/go.mod h1:IBiWB2sKIp3wVVQ3Y035++gc+knqhUQag1KpM8ahLw8=
301301
github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
302302
github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
303+
github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw=
303304
github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs=
304305
github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
305306
github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4=
@@ -308,6 +309,7 @@ github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/
308309
github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
309310
github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
310311
github.com/stretchr/testify v1.8.0 h1:pSgiaMZlXftHpm5L7V1+rVB+AZJydKsMxsQBIJw4PKk=
312+
github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU=
311313
github.com/xanzy/ssh-agent v0.3.0/go.mod h1:3s9xbODqPuuhK9JV1R321M/FlMZSBvE5aY6eAcqrDh0=
312314
github.com/yuin/goldmark v1.1.25/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
313315
github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=

‎internal/k8sutil/k8sutil.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
package k8sutil
2+
3+
import (
4+
"regexp"
5+
6+
"k8s.io/apimachinery/pkg/util/validation"
7+
)
8+
9+
var invalidNameChars = regexp.MustCompile(`[^\.\-a-zA-Z0-9]`)
10+
11+
// MetadataName replaces all invalid DNS characters with a dash. If the result
12+
// is not a valid DNS subdomain, returns `result, false`. Otherwise, returns the
13+
// `result, true`.
14+
func MetadataName(name string) (string, bool) {
15+
result := invalidNameChars.ReplaceAllString(name, "-")
16+
return result, validation.IsDNS1123Subdomain(result) == nil
17+
}

‎internal/k8sutil/k8sutil_test.go

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
package k8sutil
2+
3+
import (
4+
"fmt"
5+
"strings"
6+
"testing"
7+
8+
"github.com/stretchr/testify/assert"
9+
)
10+
11+
func TestMetadataName(t *testing.T) {
12+
type testCase struct {
13+
name string
14+
in string
15+
expectedResult string
16+
expectedValid bool
17+
}
18+
for _, tc := range []testCase{
19+
{
20+
name: "empty",
21+
in: "",
22+
expectedResult: "",
23+
expectedValid: false,
24+
},
25+
{
26+
name: "invalid",
27+
in: "foo-bar.123!",
28+
expectedResult: "foo-bar.123-",
29+
expectedValid: false,
30+
},
31+
{
32+
name: "too long",
33+
in: fmt.Sprintf("foo-bar_%s", strings.Repeat("1234567890", 50)),
34+
expectedResult: fmt.Sprintf("foo-bar-%s", strings.Repeat("1234567890", 50)),
35+
expectedValid: false,
36+
},
37+
{
38+
name: "valid",
39+
in: "foo-bar.123",
40+
expectedResult: "foo-bar.123",
41+
expectedValid: true,
42+
},
43+
{
44+
name: "valid with underscore",
45+
in: "foo-bar_123",
46+
expectedResult: "foo-bar-123",
47+
expectedValid: true,
48+
},
49+
{
50+
name: "valid with colon",
51+
in: "foo-bar:123",
52+
expectedResult: "foo-bar-123",
53+
expectedValid: true,
54+
},
55+
} {
56+
t.Run(tc.name, func(t *testing.T) {
57+
actualResult, actualValid := MetadataName(tc.in)
58+
assert.Equal(t, tc.expectedResult, actualResult)
59+
assert.Equal(t, tc.expectedValid, actualValid)
60+
})
61+
}
62+
}

‎pkg/controllers/core/catalog_controller.go

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,14 @@ package core
1818

1919
import (
2020
"context"
21-
// #nosec
22-
"crypto/sha1"
21+
"crypto/sha1" // #nosec
2322
"encoding/hex"
2423
"encoding/json"
2524
"fmt"
2625
"io/fs"
2726

2827
"github.com/operator-framework/operator-registry/alpha/declcfg"
28+
"github.com/operator-framework/operator-registry/alpha/property"
2929
corev1 "k8s.io/api/core/v1"
3030
"k8s.io/apimachinery/pkg/api/equality"
3131
"k8s.io/apimachinery/pkg/api/meta"
@@ -40,6 +40,7 @@ import (
4040
"sigs.k8s.io/controller-runtime/pkg/predicate"
4141

4242
"github.com/operator-framework/catalogd/api/core/v1alpha1"
43+
"github.com/operator-framework/catalogd/internal/k8sutil"
4344
"github.com/operator-framework/catalogd/internal/source"
4445
"github.com/operator-framework/catalogd/pkg/features"
4546
)
@@ -223,7 +224,7 @@ func (r *CatalogReconciler) syncBundleMetadata(ctx context.Context, declCfg *dec
223224
newBundles := map[string]*v1alpha1.BundleMetadata{}
224225

225226
for _, bundle := range declCfg.Bundles {
226-
bundleName := fmt.Sprintf("%s-%s", catalog.Name, bundle.Name)
227+
bundleName, _ := k8sutil.MetadataName(fmt.Sprintf("%s-%s", catalog.Name, bundle.Name))
227228

228229
bundleMeta := v1alpha1.BundleMetadata{
229230
TypeMeta: metav1.TypeMeta{
@@ -303,7 +304,7 @@ func (r *CatalogReconciler) syncPackages(ctx context.Context, declCfg *declcfg.D
303304
newPkgs := map[string]*v1alpha1.Package{}
304305

305306
for _, pkg := range declCfg.Packages {
306-
name := fmt.Sprintf("%s-%s", catalog.Name, pkg.Name)
307+
name, _ := k8sutil.MetadataName(fmt.Sprintf("%s-%s", catalog.Name, pkg.Name))
307308
var icon *v1alpha1.Icon
308309
if pkg.Icon != nil {
309310
icon = &v1alpha1.Icon{
@@ -342,7 +343,7 @@ func (r *CatalogReconciler) syncPackages(ctx context.Context, declCfg *declcfg.D
342343
}
343344

344345
for _, ch := range declCfg.Channels {
345-
pkgName := fmt.Sprintf("%s-%s", catalog.Name, ch.Package)
346+
pkgName, _ := k8sutil.MetadataName(fmt.Sprintf("%s-%s", catalog.Name, ch.Package))
346347
pkg, ok := newPkgs[pkgName]
347348
if !ok {
348349
return fmt.Errorf("channel %q references package %q which does not exist", ch.Name, ch.Package)
@@ -404,6 +405,26 @@ func (r *CatalogReconciler) syncCatalogMetadata(ctx context.Context, fsys fs.FS,
404405
return fmt.Errorf("error in generating catalog metadata name: %w", err)
405406
}
406407

408+
blob := meta.Blob
409+
if meta.Schema == declcfg.SchemaBundle {
410+
var b declcfg.Bundle
411+
if err := json.Unmarshal(blob, &b); err != nil {
412+
return fmt.Errorf("error unmarshalling bundle: %w", err)
413+
}
414+
properties := b.Properties[:0]
415+
for _, p := range b.Properties {
416+
if p.Type == property.TypeBundleObject {
417+
continue
418+
}
419+
properties = append(properties, p)
420+
}
421+
b.Properties = properties
422+
blob, err = json.Marshal(b)
423+
if err != nil {
424+
return fmt.Errorf("error marshalling bundle: %w", err)
425+
}
426+
}
427+
407428
catalogMetadata := &v1alpha1.CatalogMetadata{
408429
TypeMeta: metav1.TypeMeta{
409430
APIVersion: v1alpha1.GroupVersion.String(),
@@ -432,7 +453,7 @@ func (r *CatalogReconciler) syncCatalogMetadata(ctx context.Context, fsys fs.FS,
432453
Name: meta.Name,
433454
Schema: meta.Schema,
434455
Package: meta.Package,
435-
Content: meta.Blob,
456+
Content: blob,
436457
},
437458
}
438459

@@ -474,6 +495,7 @@ func (r *CatalogReconciler) syncCatalogMetadata(ctx context.Context, fsys fs.FS,
474495
// In the place of the empty `meta.Name`, it computes a hash of `meta.Blob` to prevent multiple FBC blobs colliding on the the object name.
475496
// Possible outcomes are: "{catalogName}-{meta.Schema}-{meta.Name}", "{catalogName}-{meta.Schema}-{meta.Package}-{meta.Name}",
476497
// "{catalogName}-{meta.Schema}-{hash{meta.Blob}}", "{catalogName}-{meta.Schema}-{meta.Package}-{hash{meta.Blob}}".
498+
// Characters that would result in an invalid DNS name are replaced with dashes.
477499
func generateCatalogMetadataName(_ context.Context, catalogName string, meta *declcfg.Meta) (string, error) {
478500
objName := fmt.Sprintf("%s-%s", catalogName, meta.Schema)
479501
if meta.Package != "" {
@@ -491,5 +513,6 @@ func generateCatalogMetadataName(_ context.Context, catalogName string, meta *de
491513
h.Write(metaJSON)
492514
objName = fmt.Sprintf("%s-%s", objName, hex.EncodeToString(h.Sum(nil)))
493515
}
516+
objName, _ = k8sutil.MetadataName(objName)
494517
return objName, nil
495518
}

‎pkg/controllers/core/catalog_controller_test.go

Lines changed: 11 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919
"sigs.k8s.io/controller-runtime/pkg/manager"
2020

2121
"github.com/operator-framework/catalogd/api/core/v1alpha1"
22+
"github.com/operator-framework/catalogd/internal/k8sutil"
2223
"github.com/operator-framework/catalogd/internal/source"
2324
"github.com/operator-framework/catalogd/pkg/controllers/core"
2425
"github.com/operator-framework/catalogd/pkg/features"
@@ -222,14 +223,14 @@ var _ = Describe("Catalogd Controller Test", func() {
222223

223224
When("unpacker returns source.Result with state == 'Unpacked'", func() {
224225
var (
225-
testBundleName = "webhook-operator.v0.0.1"
226+
testBundleName = "webhook_operator.v0.0.1"
226227
testBundleImage = "quay.io/olmtest/webhook-operator-bundle:0.0.3"
227228
testBundleRelatedImageName = "test"
228229
testBundleRelatedImageImage = "testimage:latest"
229230
testBundleObjectData = "dW5pbXBvcnRhbnQK"
230-
testPackageDefaultChannel = "preview"
231-
testPackageName = "webhook-operator"
232-
testChannelName = "preview"
231+
testPackageDefaultChannel = "preview_test"
232+
testPackageName = "webhook_operator_test"
233+
testChannelName = "preview_test"
233234
testPackage = fmt.Sprintf(testPackageTemplate, testPackageDefaultChannel, testPackageName)
234235
testBundle = fmt.Sprintf(testBundleTemplate, testBundleImage, testBundleName, testPackageName, testBundleRelatedImageName, testBundleRelatedImageImage, testBundleObjectData)
235236
testChannel = fmt.Sprintf(testChannelTemplate, testPackageName, testChannelName, testBundleName)
@@ -238,8 +239,8 @@ var _ = Describe("Catalogd Controller Test", func() {
238239
testPackageMetaName string
239240
)
240241
BeforeEach(func() {
241-
testBundleMetaName = fmt.Sprintf("%s-%s", catalog.Name, testBundleName)
242-
testPackageMetaName = fmt.Sprintf("%s-%s", catalog.Name, testPackageName)
242+
testBundleMetaName, _ = k8sutil.MetadataName(fmt.Sprintf("%s-%s", catalog.Name, testBundleName))
243+
testPackageMetaName, _ = k8sutil.MetadataName(fmt.Sprintf("%s-%s", catalog.Name, testPackageName))
243244

244245
filesys := &fstest.MapFS{
245246
"bundle.yaml": &fstest.MapFile{Data: []byte(testBundle), Mode: os.ModePerm},
@@ -286,21 +287,9 @@ var _ = Describe("Catalogd Controller Test", func() {
286287
})
287288

288289
AfterEach(func() {
289-
// clean up package
290-
pkg := &v1alpha1.Package{
291-
ObjectMeta: metav1.ObjectMeta{
292-
Name: testPackageMetaName,
293-
},
294-
}
295-
Expect(cl.Delete(ctx, pkg)).To(Succeed())
290+
Expect(cl.DeleteAllOf(ctx, &v1alpha1.Package{})).To(Succeed())
291+
Expect(cl.DeleteAllOf(ctx, &v1alpha1.BundleMetadata{})).To(Succeed())
296292

297-
// clean up bundlemetadata
298-
bm := &v1alpha1.BundleMetadata{
299-
ObjectMeta: metav1.ObjectMeta{
300-
Name: testBundleMetaName,
301-
},
302-
}
303-
Expect(cl.Delete(ctx, bm)).To(Succeed())
304293
Expect(features.CatalogdFeatureGate.SetFromMap(map[string]bool{
305294
string(features.PackagesBundleMetadataAPIs): false,
306295
string(features.CatalogMetadataAPI): false,
@@ -358,8 +347,8 @@ var _ = Describe("Catalogd Controller Test", func() {
358347
},
359348
}
360349

361-
tempTestBundleMetaName = fmt.Sprintf("%s-%s", tempCatalog.Name, testBundleName)
362-
tempTestPackageMetaName = fmt.Sprintf("%s-%s", tempCatalog.Name, testPackageName)
350+
tempTestBundleMetaName, _ = k8sutil.MetadataName(fmt.Sprintf("%s-%s", tempCatalog.Name, testBundleName))
351+
tempTestPackageMetaName, _ = k8sutil.MetadataName(fmt.Sprintf("%s-%s", tempCatalog.Name, testPackageName))
363352

364353
Expect(cl.Create(ctx, tempCatalog)).To(Succeed())
365354
res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: types.NamespacedName{Name: "tempedout"}})

0 commit comments

Comments
 (0)