Skip to content

Commit 6c2be08

Browse files
grokspawngrokspawn
authored
⚠️ updates from api audit (#1404)
* updates from api audit Signed-off-by: Jordan Keister <jordan@nimblewidget.com> * review updates Signed-off-by: Jordan Keister <jordan@nimblewidget.com> * e2e fixes Signed-off-by: Jordan Keister <jordan@nimblewidget.com> --------- Signed-off-by: Jordan Keister <jordan@nimblewidget.com>
1 parent d0865da commit 6c2be08

13 files changed

+386
-350
lines changed

‎api/v1alpha1/clusterextension_types.go

Lines changed: 147 additions & 159 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: 107 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(ctx context.Context, preflight Preflight, 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(ctx, preflight, 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/controllers/clusterextension_controller_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -687,7 +687,7 @@ func TestClusterExtensionInstallationSucceeds(t *testing.T) {
687687
t.Log("By checking the expected progressing conditions")
688688
progressingCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeProgressing)
689689
require.NotNil(t, progressingCond)
690-
require.Equal(t, metav1.ConditionFalse, progressingCond.Status)
690+
require.Equal(t, metav1.ConditionTrue, progressingCond.Status)
691691
require.Equal(t, ocv1alpha1.ReasonSucceeded, progressingCond.Reason)
692692

693693
require.NoError(t, cl.DeleteAllOf(ctx, &ocv1alpha1.ClusterExtension{}))

‎internal/controllers/common_controller.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,14 +83,13 @@ func setInstallStatus(ext *ocv1alpha1.ClusterExtension, installStatus *ocv1alpha
8383
func setStatusProgressing(ext *ocv1alpha1.ClusterExtension, err error) {
8484
progressingCond := metav1.Condition{
8585
Type: ocv1alpha1.TypeProgressing,
86-
Status: metav1.ConditionFalse,
86+
Status: metav1.ConditionTrue,
8787
Reason: ocv1alpha1.ReasonSucceeded,
8888
Message: "desired state reached",
8989
ObservedGeneration: ext.GetGeneration(),
9090
}
9191

9292
if err != nil {
93-
progressingCond.Status = metav1.ConditionTrue
9493
progressingCond.Reason = ocv1alpha1.ReasonRetrying
9594
progressingCond.Message = err.Error()
9695
}

‎internal/controllers/common_controller_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,12 @@ func TestSetStatusProgressing(t *testing.T) {
2222
expected metav1.Condition
2323
}{
2424
{
25-
name: "non-nil ClusterExtension, nil error, Progressing condition has status False with reason Success",
25+
name: "non-nil ClusterExtension, nil error, Progressing condition has status True with reason Success",
2626
err: nil,
2727
clusterExtension: &ocv1alpha1.ClusterExtension{},
2828
expected: metav1.Condition{
2929
Type: ocv1alpha1.TypeProgressing,
30-
Status: metav1.ConditionFalse,
30+
Status: metav1.ConditionTrue,
3131
Reason: ocv1alpha1.ReasonSucceeded,
3232
Message: "desired state reached",
3333
},

0 commit comments

Comments
 (0)