Skip to content

Commit fd0ce87

Browse files
openshift-merge-robotopenshift-merge-robot
authored
Merge pull request #1993 from benluddy/package-name-inference
Bug 1921953: Infer package name property for unannotated CSVs, if possible.
2 parents ac4e989 + 5273d09 commit fd0ce87

File tree

4 files changed

+318
-11
lines changed

4 files changed

+318
-11
lines changed

‎pkg/controller/registry/resolver/resolver.go

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"fmt"
77
"sort"
88

9+
"github.com/blang/semver/v4"
910
"github.com/sirupsen/logrus"
1011
utilerrors "k8s.io/apimachinery/pkg/util/errors"
1112

@@ -14,6 +15,7 @@ import (
1415
"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry"
1516
"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/resolver/projection"
1617
"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/resolver/solver"
18+
"github.com/operator-framework/operator-registry/pkg/api"
1719
opregistry "github.com/operator-framework/operator-registry/pkg/registry"
1820
)
1921

@@ -362,6 +364,51 @@ func (r *SatResolver) getBundleInstallables(catalog registry.CatalogKey, predica
362364
return ids, installables, nil
363365
}
364366

367+
func (r *SatResolver) inferProperties(csv *v1alpha1.ClusterServiceVersion, subs []*v1alpha1.Subscription) ([]*api.Property, error) {
368+
var properties []*api.Property
369+
370+
packages := make(map[string]struct{})
371+
for _, sub := range subs {
372+
if sub.Status.InstalledCSV != csv.Name {
373+
continue
374+
}
375+
// Without sanity checking the Subscription spec's
376+
// package against catalog contents, updates to the
377+
// Subscription spec could result in a bad package
378+
// inference.
379+
for _, entry := range r.cache.Namespaced(sub.Namespace).Catalog(registry.CatalogKey{Namespace: sub.Spec.CatalogSourceNamespace, Name: sub.Spec.CatalogSource}).Find(And(WithCSVName(csv.Name), WithPackage(sub.Spec.Package))) {
380+
if pkg := entry.Package(); pkg != "" {
381+
packages[pkg] = struct{}{}
382+
}
383+
}
384+
}
385+
if l := len(packages); l != 1 {
386+
r.log.Warnf("could not unambiguously infer package name for %q (found %d distinct package names)", csv.Name, l)
387+
return properties, nil
388+
}
389+
var pkg string
390+
for pkg = range packages {
391+
// Assign the single key to pkg.
392+
}
393+
var version string // Emit empty string rather than "0.0.0" if .spec.version is zero-valued.
394+
if !csv.Spec.Version.Version.Equals(semver.Version{}) {
395+
version = csv.Spec.Version.String()
396+
}
397+
if pp, err := json.Marshal(opregistry.PackageProperty{
398+
PackageName: pkg,
399+
Version: version,
400+
}); err != nil {
401+
return nil, fmt.Errorf("failed to marshal inferred package property: %w", err)
402+
} else {
403+
properties = append(properties, &api.Property{
404+
Type: opregistry.PackageType,
405+
Value: string(pp),
406+
})
407+
}
408+
409+
return properties, nil
410+
}
411+
365412
func (r *SatResolver) newSnapshotForNamespace(namespace string, subs []*v1alpha1.Subscription, csvs []*v1alpha1.ClusterServiceVersion) (*CatalogSnapshot, []solver.Installable, error) {
366413
installables := make([]solver.Installable, 0)
367414
existingOperatorCatalog := registry.NewVirtualCatalogKey(namespace)
@@ -393,6 +440,11 @@ func (r *SatResolver) newSnapshotForNamespace(namespace string, subs []*v1alpha1
393440

394441
if anno, ok := csv.GetAnnotations()[projection.PropertiesAnnotationKey]; !ok {
395442
csvsMissingProperties = append(csvsMissingProperties, csv)
443+
if inferred, err := r.inferProperties(csv, subs); err != nil {
444+
r.log.Warnf("unable to infer properties for csv %q: %w", csv.Name, err)
445+
} else {
446+
op.properties = append(op.properties, inferred...)
447+
}
396448
} else if props, err := projection.PropertyListFromPropertiesAnnotation(anno); err != nil {
397449
return nil, nil, fmt.Errorf("failed to retrieve properties of csv %q: %w", csv.GetName(), err)
398450
} else {

‎pkg/controller/registry/resolver/resolver_test.go

Lines changed: 209 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,18 @@ import (
66

77
"github.com/blang/semver/v4"
88
"github.com/sirupsen/logrus"
9+
"github.com/sirupsen/logrus/hooks/test"
910
"github.com/stretchr/testify/assert"
1011
"github.com/stretchr/testify/require"
1112

12-
"github.com/operator-framework/api/pkg/operators/v1alpha1"
13-
"github.com/operator-framework/operator-registry/pkg/api"
14-
opregistry "github.com/operator-framework/operator-registry/pkg/registry"
13+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1514

15+
"github.com/operator-framework/api/pkg/lib/version"
16+
"github.com/operator-framework/api/pkg/operators/v1alpha1"
1617
"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry"
1718
"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/resolver/solver"
19+
"github.com/operator-framework/operator-registry/pkg/api"
20+
opregistry "github.com/operator-framework/operator-registry/pkg/registry"
1821
)
1922

2023
func TestSolveOperators(t *testing.T) {
@@ -1456,3 +1459,206 @@ func TestSolveOperators_WithSkips(t *testing.T) {
14561459
}
14571460
require.EqualValues(t, expected, operators)
14581461
}
1462+
1463+
func TestInferProperties(t *testing.T) {
1464+
catalog := registry.CatalogKey{Namespace: "namespace", Name: "name"}
1465+
1466+
for _, tc := range []struct {
1467+
Name string
1468+
Cache NamespacedOperatorCache
1469+
CSV *v1alpha1.ClusterServiceVersion
1470+
Subscriptions []*v1alpha1.Subscription
1471+
Expected []*api.Property
1472+
}{
1473+
{
1474+
Name: "no subscriptions infers no properties",
1475+
CSV: &v1alpha1.ClusterServiceVersion{
1476+
ObjectMeta: metav1.ObjectMeta{
1477+
Name: "a",
1478+
},
1479+
},
1480+
},
1481+
{
1482+
Name: "one unrelated subscription infers no properties",
1483+
CSV: &v1alpha1.ClusterServiceVersion{
1484+
ObjectMeta: metav1.ObjectMeta{
1485+
Name: "a",
1486+
},
1487+
},
1488+
Subscriptions: []*v1alpha1.Subscription{
1489+
{
1490+
Spec: &v1alpha1.SubscriptionSpec{
1491+
Package: "x",
1492+
},
1493+
Status: v1alpha1.SubscriptionStatus{
1494+
InstalledCSV: "b",
1495+
},
1496+
},
1497+
},
1498+
},
1499+
{
1500+
Name: "one subscription with empty package field infers no properties",
1501+
CSV: &v1alpha1.ClusterServiceVersion{
1502+
ObjectMeta: metav1.ObjectMeta{
1503+
Name: "a",
1504+
},
1505+
},
1506+
Subscriptions: []*v1alpha1.Subscription{
1507+
{
1508+
Spec: &v1alpha1.SubscriptionSpec{
1509+
Package: "",
1510+
},
1511+
Status: v1alpha1.SubscriptionStatus{
1512+
InstalledCSV: "a",
1513+
},
1514+
},
1515+
},
1516+
},
1517+
{
1518+
Name: "two related subscriptions infers no properties",
1519+
CSV: &v1alpha1.ClusterServiceVersion{
1520+
ObjectMeta: metav1.ObjectMeta{
1521+
Name: "a",
1522+
},
1523+
},
1524+
Subscriptions: []*v1alpha1.Subscription{
1525+
{
1526+
Spec: &v1alpha1.SubscriptionSpec{
1527+
Package: "x",
1528+
},
1529+
Status: v1alpha1.SubscriptionStatus{
1530+
InstalledCSV: "a",
1531+
},
1532+
},
1533+
{
1534+
Spec: &v1alpha1.SubscriptionSpec{
1535+
Package: "x",
1536+
},
1537+
Status: v1alpha1.SubscriptionStatus{
1538+
InstalledCSV: "a",
1539+
},
1540+
},
1541+
},
1542+
},
1543+
{
1544+
Name: "one matching subscription infers package property",
1545+
Cache: NamespacedOperatorCache{
1546+
snapshots: map[registry.CatalogKey]*CatalogSnapshot{
1547+
catalog: {
1548+
key: catalog,
1549+
operators: []*Operator{
1550+
{
1551+
name: "a",
1552+
bundle: &api.Bundle{
1553+
PackageName: "x",
1554+
},
1555+
},
1556+
},
1557+
},
1558+
},
1559+
},
1560+
CSV: &v1alpha1.ClusterServiceVersion{
1561+
ObjectMeta: metav1.ObjectMeta{
1562+
Name: "a",
1563+
},
1564+
Spec: v1alpha1.ClusterServiceVersionSpec{
1565+
Version: version.OperatorVersion{Version: semver.MustParse("1.2.3")},
1566+
},
1567+
},
1568+
Subscriptions: []*v1alpha1.Subscription{
1569+
{
1570+
Spec: &v1alpha1.SubscriptionSpec{
1571+
Package: "x",
1572+
CatalogSource: catalog.Name,
1573+
CatalogSourceNamespace: catalog.Namespace,
1574+
},
1575+
Status: v1alpha1.SubscriptionStatus{
1576+
InstalledCSV: "a",
1577+
},
1578+
},
1579+
},
1580+
Expected: []*api.Property{
1581+
{
1582+
Type: "olm.package",
1583+
Value: `{"packageName":"x","version":"1.2.3"}`,
1584+
},
1585+
},
1586+
},
1587+
{
1588+
Name: "one matching subscription without catalog entry infers no properties",
1589+
CSV: &v1alpha1.ClusterServiceVersion{
1590+
ObjectMeta: metav1.ObjectMeta{
1591+
Name: "a",
1592+
},
1593+
Spec: v1alpha1.ClusterServiceVersionSpec{
1594+
Version: version.OperatorVersion{Version: semver.MustParse("1.2.3")},
1595+
},
1596+
},
1597+
Subscriptions: []*v1alpha1.Subscription{
1598+
{
1599+
Spec: &v1alpha1.SubscriptionSpec{
1600+
Package: "x",
1601+
},
1602+
Status: v1alpha1.SubscriptionStatus{
1603+
InstalledCSV: "a",
1604+
},
1605+
},
1606+
},
1607+
},
1608+
{
1609+
Name: "one matching subscription infers package property without csv version",
1610+
Cache: NamespacedOperatorCache{
1611+
snapshots: map[registry.CatalogKey]*CatalogSnapshot{
1612+
catalog: {
1613+
key: catalog,
1614+
operators: []*Operator{
1615+
{
1616+
name: "a",
1617+
bundle: &api.Bundle{
1618+
PackageName: "x",
1619+
},
1620+
},
1621+
},
1622+
},
1623+
},
1624+
},
1625+
CSV: &v1alpha1.ClusterServiceVersion{
1626+
ObjectMeta: metav1.ObjectMeta{
1627+
Name: "a",
1628+
},
1629+
},
1630+
Subscriptions: []*v1alpha1.Subscription{
1631+
{
1632+
Spec: &v1alpha1.SubscriptionSpec{
1633+
Package: "x",
1634+
CatalogSource: catalog.Name,
1635+
CatalogSourceNamespace: catalog.Namespace,
1636+
},
1637+
Status: v1alpha1.SubscriptionStatus{
1638+
InstalledCSV: "a",
1639+
},
1640+
},
1641+
},
1642+
Expected: []*api.Property{
1643+
{
1644+
Type: "olm.package",
1645+
Value: `{"packageName":"x","version":""}`,
1646+
},
1647+
},
1648+
},
1649+
} {
1650+
t.Run(tc.Name, func(t *testing.T) {
1651+
require := require.New(t)
1652+
logger, _ := test.NewNullLogger()
1653+
r := SatResolver{
1654+
log: logger,
1655+
cache: &FakeOperatorCache{
1656+
fakedNamespacedOperatorCache: tc.Cache,
1657+
},
1658+
}
1659+
actual, err := r.inferProperties(tc.CSV, tc.Subscriptions)
1660+
require.NoError(err)
1661+
require.Equal(tc.Expected, actual)
1662+
})
1663+
}
1664+
}

‎test/e2e/e2e_test.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,8 @@ func TestEndToEnd(t *testing.T) {
4848
RegisterFailHandler(Fail)
4949
SetDefaultEventuallyTimeout(1 * time.Minute)
5050
SetDefaultEventuallyPollingInterval(1 * time.Second)
51+
SetDefaultConsistentlyDuration(30 * time.Second)
52+
SetDefaultConsistentlyPollingInterval(1 * time.Second)
5153

5254
if junitDir := os.Getenv("JUNIT_DIRECTORY"); junitDir != "" {
5355
junitReporter := reporters.NewJUnitReporter(path.Join(junitDir, fmt.Sprintf("junit_e2e_%02d.xml", config.GinkgoConfig.ParallelNode)))

0 commit comments

Comments
 (0)