Skip to content

Commit f04b254

Browse files
grokspawngrokspawn
committed
updates from api audit
Signed-off-by: Jordan Keister <[email protected]>
1 parent 6f42274 commit f04b254

File tree

9 files changed

+228
-205
lines changed

9 files changed

+228
-205
lines changed

‎api/v1alpha1/clusterextension_types.go‎

Lines changed: 83 additions & 80 deletions
Large diffs are not rendered by default.

‎api/v1alpha1/zz_generated.deepcopy.go‎

Lines changed: 5 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎config/base/crd/bases/olm.operatorframework.io_clusterextensions.yaml‎

Lines changed: 67 additions & 76 deletions
Large diffs are not rendered by default.

‎docs/api-reference/operator-controller-api-reference.md‎

Lines changed: 22 additions & 22 deletions
Large diffs are not rendered by default.

‎internal/applier/helm.go‎

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
2020
apimachyaml "k8s.io/apimachinery/pkg/util/yaml"
2121
"sigs.k8s.io/controller-runtime/pkg/client"
22+
"sigs.k8s.io/controller-runtime/pkg/log"
2223

2324
helmclient "github.com/operator-framework/helm-operator-plugins/pkg/client"
2425

@@ -56,6 +57,25 @@ type Helm struct {
5657
Preflights []Preflight
5758
}
5859

60+
// shouldSkipPreflight is a helper to determine if the preflight check is CRDUpgradeSafety AND
61+
// if it is set to enforcement None.
62+
func shouldSkipPreflight(preflight Preflight, ctx context.Context, ext *ocv1alpha1.ClusterExtension, state string) bool {
63+
l := log.FromContext(ctx)
64+
if ext.Spec.Install.Preflight != nil && ext.Spec.Install.Preflight.CRDUpgradeSafety != nil {
65+
if _, ok := preflight.(*crdupgradesafety.Preflight); ok {
66+
if state == StateNeedsInstall || state == StateNeedsUpgrade {
67+
l.Info("crdUpgradeSafety ", "policy", ext.Spec.Install.Preflight.CRDUpgradeSafety.Enforcement)
68+
}
69+
if ext.Spec.Install.Preflight.CRDUpgradeSafety.Enforcement == ocv1alpha1.CRDUpgradeSafetyEnforcementNone {
70+
// Skip this preflight check because it is of type *crdupgradesafety.Preflight and the CRD Upgrade Safety
71+
// policy is set to None
72+
return true
73+
}
74+
}
75+
}
76+
return false
77+
}
78+
5979
func (h *Helm) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1alpha1.ClusterExtension, objectLabels map[string]string, storageLabels map[string]string) ([]client.Object, string, error) {
6080
chrt, err := convert.RegistryV1ToHelmChart(ctx, contentFS, ext.Spec.Install.Namespace, []string{corev1.NamespaceAll})
6181
if err != nil {
@@ -78,12 +98,8 @@ func (h *Helm) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1alpha1.Clust
7898
}
7999

80100
for _, preflight := range h.Preflights {
81-
if ext.Spec.Install.Preflight != nil && ext.Spec.Install.Preflight.CRDUpgradeSafety != nil {
82-
if _, ok := preflight.(*crdupgradesafety.Preflight); ok && ext.Spec.Install.Preflight.CRDUpgradeSafety.Policy == ocv1alpha1.CRDUpgradeSafetyPolicyDisabled {
83-
// Skip this preflight check because it is of type *crdupgradesafety.Preflight and the CRD Upgrade Safety
84-
// preflight check has been disabled
85-
continue
86-
}
101+
if shouldSkipPreflight(preflight, ctx, ext, state) {
102+
continue
87103
}
88104
switch state {
89105
case StateNeedsInstall:

‎internal/controllers/clusterextension_admission_test.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ func TestClusterExtensionSourceConfig(t *testing.T) {
7777

7878
func TestClusterExtensionAdmissionPackageName(t *testing.T) {
7979
tooLongError := "spec.source.catalog.packageName: Too long: may not be longer than 253"
80-
regexMismatchError := "spec.source.catalog.packageName in body should match"
80+
regexMismatchError := "packageName must be a valid DNS1123 subdomain"
8181

8282
testCases := []struct {
8383
name string
@@ -136,7 +136,7 @@ func TestClusterExtensionAdmissionPackageName(t *testing.T) {
136136

137137
func TestClusterExtensionAdmissionVersion(t *testing.T) {
138138
tooLongError := "spec.source.catalog.version: Too long: may not be longer than 64"
139-
regexMismatchError := "spec.source.catalog.version in body should match"
139+
regexMismatchError := "invalid version expression in the catalog source"
140140

141141
testCases := []struct {
142142
name string
@@ -293,7 +293,7 @@ func TestClusterExtensionAdmissionChannel(t *testing.T) {
293293

294294
func TestClusterExtensionAdmissionInstallNamespace(t *testing.T) {
295295
tooLongError := "spec.install.namespace: Too long: may not be longer than 63"
296-
regexMismatchError := "spec.install.namespace in body should match"
296+
regexMismatchError := "namespace must be a valid DNS1123 label"
297297

298298
testCases := []struct {
299299
name string

‎internal/resolve/catalog.go‎

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -43,13 +43,18 @@ func (r *CatalogResolver) Resolve(ctx context.Context, ext *ocv1alpha1.ClusterEx
4343
versionRange := ext.Spec.Source.Catalog.Version
4444
channels := ext.Spec.Source.Catalog.Channels
4545

46-
selector, err := metav1.LabelSelectorAsSelector(&ext.Spec.Source.Catalog.Selector)
47-
if err != nil {
48-
return nil, nil, nil, fmt.Errorf("desired catalog selector is invalid: %w", err)
49-
}
50-
// A nothing (empty) seletor selects everything
51-
if selector == labels.Nothing() {
52-
selector = labels.Everything()
46+
// unless overridden, default to selecting all bundles
47+
var selector labels.Selector = labels.Everything()
48+
var err error
49+
if ext.Spec.Source.Catalog != nil {
50+
selector, err = metav1.LabelSelectorAsSelector(ext.Spec.Source.Catalog.Selector)
51+
if err != nil {
52+
return nil, nil, nil, fmt.Errorf("desired catalog selector is invalid: %w", err)
53+
}
54+
// A nothing (empty) selector selects everything
55+
if selector == labels.Nothing() {
56+
selector = labels.Everything()
57+
}
5358
}
5459

5560
var versionRangeConstraints *mmsemver.Constraints

‎internal/resolve/catalog_test.go‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -770,7 +770,7 @@ func TestInvalidClusterExtensionCatalogMatchExpressions(t *testing.T) {
770770
Source: ocv1alpha1.SourceConfig{
771771
Catalog: &ocv1alpha1.CatalogSource{
772772
PackageName: "foo",
773-
Selector: metav1.LabelSelector{
773+
Selector: &metav1.LabelSelector{
774774
MatchExpressions: []metav1.LabelSelectorRequirement{
775775
{
776776
Key: "name",
@@ -802,7 +802,7 @@ func TestInvalidClusterExtensionCatalogMatchLabelsName(t *testing.T) {
802802
Source: ocv1alpha1.SourceConfig{
803803
Catalog: &ocv1alpha1.CatalogSource{
804804
PackageName: "foo",
805-
Selector: metav1.LabelSelector{
805+
Selector: &metav1.LabelSelector{
806806
MatchLabels: map[string]string{"": "value"},
807807
},
808808
},
@@ -828,7 +828,7 @@ func TestInvalidClusterExtensionCatalogMatchLabelsValue(t *testing.T) {
828828
Source: ocv1alpha1.SourceConfig{
829829
Catalog: &ocv1alpha1.CatalogSource{
830830
PackageName: "foo",
831-
Selector: metav1.LabelSelector{
831+
Selector: &metav1.LabelSelector{
832832
MatchLabels: map[string]string{"name": "&value"},
833833
},
834834
},
@@ -852,7 +852,9 @@ func TestClusterExtensionMatchLabel(t *testing.T) {
852852
}
853853
r := CatalogResolver{WalkCatalogsFunc: w.WalkCatalogs}
854854
ce := buildFooClusterExtension(pkgName, []string{}, "", ocv1alpha1.UpgradeConstraintPolicyCatalogProvided)
855-
ce.Spec.Source.Catalog.Selector.MatchLabels = map[string]string{"olm.operatorframework.io/metadata.name": "b"}
855+
ce.Spec.Source.Catalog.Selector = &metav1.LabelSelector{
856+
MatchLabels: map[string]string{"olm.operatorframework.io/metadata.name": "b"},
857+
}
856858

857859
_, _, _, err := r.Resolve(context.Background(), ce, nil)
858860
require.NoError(t, err)
@@ -871,7 +873,9 @@ func TestClusterExtensionNoMatchLabel(t *testing.T) {
871873
}
872874
r := CatalogResolver{WalkCatalogsFunc: w.WalkCatalogs}
873875
ce := buildFooClusterExtension(pkgName, []string{}, "", ocv1alpha1.UpgradeConstraintPolicyCatalogProvided)
874-
ce.Spec.Source.Catalog.Selector.MatchLabels = map[string]string{"olm.operatorframework.io/metadata.name": "a"}
876+
ce.Spec.Source.Catalog.Selector = &metav1.LabelSelector{
877+
MatchLabels: map[string]string{"olm.operatorframework.io/metadata.name": "a"},
878+
}
875879

876880
_, _, _, err := r.Resolve(context.Background(), ce, nil)
877881
require.Error(t, err)

‎test/e2e/cluster_extension_install_test.go‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -249,7 +249,7 @@ func TestClusterExtensionInstallRegistry(t *testing.T) {
249249
SourceType: "Catalog",
250250
Catalog: &ocv1alpha1.CatalogSource{
251251
PackageName: tc.packageName,
252-
Selector: metav1.LabelSelector{
252+
Selector: &metav1.LabelSelector{
253253
MatchLabels: map[string]string{"olm.operatorframework.io/metadata.name": extensionCatalog.Name},
254254
},
255255
},
@@ -516,7 +516,7 @@ func TestClusterExtensionInstallReResolvesWhenCatalogIsPatched(t *testing.T) {
516516
SourceType: "Catalog",
517517
Catalog: &ocv1alpha1.CatalogSource{
518518
PackageName: "prometheus",
519-
Selector: metav1.LabelSelector{
519+
Selector: &metav1.LabelSelector{
520520
MatchExpressions: []metav1.LabelSelectorRequirement{
521521
{
522522
Key: "olm.operatorframework.io/metadata.name",
@@ -603,7 +603,7 @@ func TestClusterExtensionInstallReResolvesWhenNewCatalog(t *testing.T) {
603603
SourceType: "Catalog",
604604
Catalog: &ocv1alpha1.CatalogSource{
605605
PackageName: "prometheus",
606-
Selector: metav1.LabelSelector{
606+
Selector: &metav1.LabelSelector{
607607
MatchLabels: map[string]string{"olm.operatorframework.io/metadata.name": extensionCatalog.Name},
608608
},
609609
},
@@ -666,7 +666,7 @@ func TestClusterExtensionInstallReResolvesWhenManagedContentChanged(t *testing.T
666666
SourceType: "Catalog",
667667
Catalog: &ocv1alpha1.CatalogSource{
668668
PackageName: "prometheus",
669-
Selector: metav1.LabelSelector{
669+
Selector: &metav1.LabelSelector{
670670
MatchLabels: map[string]string{"olm.operatorframework.io/metadata.name": extensionCatalog.Name},
671671
},
672672
},
@@ -731,7 +731,7 @@ func TestClusterExtensionRecoversFromInitialInstallFailedWhenFailureFixed(t *tes
731731
SourceType: "Catalog",
732732
Catalog: &ocv1alpha1.CatalogSource{
733733
PackageName: "prometheus",
734-
Selector: metav1.LabelSelector{
734+
Selector: &metav1.LabelSelector{
735735
MatchLabels: map[string]string{"olm.operatorframework.io/metadata.name": extensionCatalog.Name},
736736
},
737737
},

0 commit comments

Comments
 (0)