Skip to content

Commit e29724d

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

File tree

10 files changed

+356
-331
lines changed

10 files changed

+356
-331
lines changed

‎api/v1alpha1/clusterextension_types.go‎

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

‎api/v1alpha1/zz_generated.deepcopy.go‎

Lines changed: 10 additions & 6 deletions
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: 96 additions & 106 deletions
Large diffs are not rendered by default.

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

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

‎internal/applier/helm.go‎

Lines changed: 23 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,26 @@ 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+
hasCRDUpgradeSafety := ext.Spec.Install.Preflight != nil && ext.Spec.Install.Preflight.CRDUpgradeSafety != nil
65+
_, isCRDUpgradeSafetyInstance := preflight.(*crdupgradesafety.Preflight)
66+
67+
if hasCRDUpgradeSafety && isCRDUpgradeSafetyInstance {
68+
if state == StateNeedsInstall || state == StateNeedsUpgrade {
69+
l.Info("crdUpgradeSafety ", "policy", ext.Spec.Install.Preflight.CRDUpgradeSafety.Enforcement)
70+
}
71+
if ext.Spec.Install.Preflight.CRDUpgradeSafety.Enforcement == ocv1alpha1.CRDUpgradeSafetyEnforcementNone {
72+
// Skip this preflight check because it is of type *crdupgradesafety.Preflight and the CRD Upgrade Safety
73+
// policy is set to None
74+
return true
75+
}
76+
}
77+
return false
78+
}
79+
5980
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) {
6081
chrt, err := convert.RegistryV1ToHelmChart(ctx, contentFS, ext.Spec.Install.Namespace, []string{corev1.NamespaceAll})
6182
if err != nil {
@@ -78,12 +99,8 @@ func (h *Helm) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1alpha1.Clust
7899
}
79100

80101
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-
}
102+
if shouldSkipPreflight(preflight, ctx, ext, state) {
103+
continue
87104
}
88105
switch state {
89106
case StateNeedsInstall:

‎internal/conditionsets/conditionsets.go‎

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,27 @@ limitations under the License.
1616

1717
package conditionsets
1818

19+
import (
20+
"github.com/operator-framework/operator-controller/api/v1alpha1"
21+
)
22+
1923
// ConditionTypes is the full set of ClusterExtension condition Types.
2024
// ConditionReasons is the full set of ClusterExtension condition Reasons.
2125
//
22-
// NOTE: These are populated by init() in api/v1alpha1/clusterextension_types.go
23-
var ConditionTypes []string
24-
var ConditionReasons []string
26+
// NOTE: unit tests in clusterextension_types_test will enforce completeness.
27+
var ConditionTypes = []string{
28+
v1alpha1.TypeInstalled,
29+
v1alpha1.TypeDeprecated,
30+
v1alpha1.TypePackageDeprecated,
31+
v1alpha1.TypeChannelDeprecated,
32+
v1alpha1.TypeBundleDeprecated,
33+
v1alpha1.TypeProgressing,
34+
}
35+
36+
var ConditionReasons = []string{
37+
v1alpha1.ReasonSucceeded,
38+
v1alpha1.ReasonDeprecated,
39+
v1alpha1.ReasonFailed,
40+
v1alpha1.ReasonBlocked,
41+
v1alpha1.ReasonRetrying,
42+
}

‎internal/controllers/clusterextension_admission_test.go‎

Lines changed: 5 additions & 5 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"
140140

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

237237
func TestClusterExtensionAdmissionChannel(t *testing.T) {
238238
tooLongError := "spec.source.catalog.channels[0]: Too long: may not be longer than 253"
239-
regexMismatchError := "spec.source.catalog.channels[0] in body should match"
239+
regexMismatchError := "channels entries must be valid DNS1123 subdomains"
240240

241241
testCases := []struct {
242242
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
@@ -348,7 +348,7 @@ func TestClusterExtensionAdmissionInstallNamespace(t *testing.T) {
348348

349349
func TestClusterExtensionAdmissionServiceAccount(t *testing.T) {
350350
tooLongError := "spec.install.serviceAccount.name: Too long: may not be longer than 253"
351-
regexMismatchError := "spec.install.serviceAccount.name in body should match"
351+
regexMismatchError := "name must be a valid DNS1123 subdomain"
352352

353353
testCases := []struct {
354354
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.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
@@ -313,7 +313,7 @@ func TestClusterExtensionInstallRegistry(t *testing.T) {
313313
SourceType: "Catalog",
314314
Catalog: &ocv1alpha1.CatalogSource{
315315
PackageName: tc.packageName,
316-
Selector: metav1.LabelSelector{
316+
Selector: &metav1.LabelSelector{
317317
MatchLabels: map[string]string{"olm.operatorframework.io/metadata.name": extensionCatalog.Name},
318318
},
319319
},
@@ -580,7 +580,7 @@ func TestClusterExtensionInstallReResolvesWhenCatalogIsPatched(t *testing.T) {
580580
SourceType: "Catalog",
581581
Catalog: &ocv1alpha1.CatalogSource{
582582
PackageName: "prometheus",
583-
Selector: metav1.LabelSelector{
583+
Selector: &metav1.LabelSelector{
584584
MatchExpressions: []metav1.LabelSelectorRequirement{
585585
{
586586
Key: "olm.operatorframework.io/metadata.name",
@@ -669,7 +669,7 @@ func TestClusterExtensionInstallReResolvesWhenNewCatalog(t *testing.T) {
669669
SourceType: "Catalog",
670670
Catalog: &ocv1alpha1.CatalogSource{
671671
PackageName: "prometheus",
672-
Selector: metav1.LabelSelector{
672+
Selector: &metav1.LabelSelector{
673673
MatchLabels: map[string]string{"olm.operatorframework.io/metadata.name": extensionCatalog.Name},
674674
},
675675
},
@@ -732,7 +732,7 @@ func TestClusterExtensionInstallReResolvesWhenManagedContentChanged(t *testing.T
732732
SourceType: "Catalog",
733733
Catalog: &ocv1alpha1.CatalogSource{
734734
PackageName: "prometheus",
735-
Selector: metav1.LabelSelector{
735+
Selector: &metav1.LabelSelector{
736736
MatchLabels: map[string]string{"olm.operatorframework.io/metadata.name": extensionCatalog.Name},
737737
},
738738
},
@@ -797,7 +797,7 @@ func TestClusterExtensionRecoversFromInitialInstallFailedWhenFailureFixed(t *tes
797797
SourceType: "Catalog",
798798
Catalog: &ocv1alpha1.CatalogSource{
799799
PackageName: "prometheus",
800-
Selector: metav1.LabelSelector{
800+
Selector: &metav1.LabelSelector{
801801
MatchLabels: map[string]string{"olm.operatorframework.io/metadata.name": extensionCatalog.Name},
802802
},
803803
},

0 commit comments

Comments
 (0)