Skip to content

Commit 4e1486d

Browse files
Fix deprecation conditions leaking install errors
- stop copying install/validation errors into deprecation conditions - base bundle deprecation on the installed bundle (Unknown when none) - extend unit tests to cover resolver failures with catalog deprecations Assisted-by: Cursor
1 parent 18142b3 commit 4e1486d

File tree

12 files changed

+431
-140
lines changed

12 files changed

+431
-140
lines changed

api/v1/clusterextension_types.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -483,12 +483,12 @@ type ClusterExtensionStatus struct {
483483
// When Progressing is True and the Reason is Retrying, the ClusterExtension has encountered an error that could be resolved on subsequent reconciliation attempts.
484484
// When Progressing is False and the Reason is Blocked, the ClusterExtension has encountered an error that requires manual intervention for recovery.
485485
//
486-
// When the ClusterExtension is sourced from a catalog, if may also communicate a deprecation condition.
486+
// When the ClusterExtension is sourced from a catalog, it may surface deprecation conditions based on catalog metadata.
487487
// These are indications from a package owner to guide users away from a particular package, channel, or bundle.
488-
// BundleDeprecated is set if the requested bundle version is marked deprecated in the catalog.
489-
// ChannelDeprecated is set if the requested channel is marked deprecated in the catalog.
490-
// PackageDeprecated is set if the requested package is marked deprecated in the catalog.
491-
// Deprecated is a rollup condition that is present when any of the deprecated conditions are present.
488+
// PackageDeprecated becomes True when the catalog marks the requested package deprecated; otherwise it stays False.
489+
// ChannelDeprecated becomes True when any requested channel is marked deprecated; otherwise it stays False.
490+
// BundleDeprecated reports the catalog status of the installed bundle: it remains Unknown until a bundle installs, then becomes True or False depending on whether the catalog marks that bundle deprecated.
491+
// Deprecated is a rollup that mirrors True whenever any of the specific deprecation conditions are True.
492492
//
493493
// +listType=map
494494
// +listMapKey=type

helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensions.yaml

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -518,12 +518,12 @@ spec:
518518
When Progressing is True and the Reason is Retrying, the ClusterExtension has encountered an error that could be resolved on subsequent reconciliation attempts.
519519
When Progressing is False and the Reason is Blocked, the ClusterExtension has encountered an error that requires manual intervention for recovery.
520520
521-
When the ClusterExtension is sourced from a catalog, if may also communicate a deprecation condition.
521+
When the ClusterExtension is sourced from a catalog, it may surface deprecation conditions based on catalog metadata.
522522
These are indications from a package owner to guide users away from a particular package, channel, or bundle.
523-
BundleDeprecated is set if the requested bundle version is marked deprecated in the catalog.
524-
ChannelDeprecated is set if the requested channel is marked deprecated in the catalog.
525-
PackageDeprecated is set if the requested package is marked deprecated in the catalog.
526-
Deprecated is a rollup condition that is present when any of the deprecated conditions are present.
523+
PackageDeprecated becomes True when the catalog marks the requested package deprecated; otherwise it stays False.
524+
ChannelDeprecated becomes True when any requested channel is marked deprecated; otherwise it stays False.
525+
BundleDeprecated reports the catalog status of the installed bundle: it remains Unknown until a bundle installs, then becomes True or False depending on whether the catalog marks that bundle deprecated.
526+
Deprecated is a rollup that mirrors True whenever any of the specific deprecation conditions are True.
527527
items:
528528
description: Condition contains details for one aspect of the current
529529
state of this API Resource.

helm/olmv1/base/operator-controller/crd/standard/olm.operatorframework.io_clusterextensions.yaml

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -479,12 +479,12 @@ spec:
479479
When Progressing is True and the Reason is Retrying, the ClusterExtension has encountered an error that could be resolved on subsequent reconciliation attempts.
480480
When Progressing is False and the Reason is Blocked, the ClusterExtension has encountered an error that requires manual intervention for recovery.
481481
482-
When the ClusterExtension is sourced from a catalog, if may also communicate a deprecation condition.
482+
When the ClusterExtension is sourced from a catalog, it may surface deprecation conditions based on catalog metadata.
483483
These are indications from a package owner to guide users away from a particular package, channel, or bundle.
484-
BundleDeprecated is set if the requested bundle version is marked deprecated in the catalog.
485-
ChannelDeprecated is set if the requested channel is marked deprecated in the catalog.
486-
PackageDeprecated is set if the requested package is marked deprecated in the catalog.
487-
Deprecated is a rollup condition that is present when any of the deprecated conditions are present.
484+
PackageDeprecated becomes True when the catalog marks the requested package deprecated; otherwise it stays False.
485+
ChannelDeprecated becomes True when any requested channel is marked deprecated; otherwise it stays False.
486+
BundleDeprecated reports the catalog status of the installed bundle: it remains Unknown until a bundle installs, then becomes True or False depending on whether the catalog marks that bundle deprecated.
487+
Deprecated is a rollup that mirrors True whenever any of the specific deprecation conditions are True.
488488
items:
489489
description: Condition contains details for one aspect of the current
490490
state of this API Resource.

internal/operator-controller/controllers/clusterextension_admission_test.go

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -13,20 +13,20 @@ import (
1313
)
1414

1515
func TestClusterExtensionSourceConfig(t *testing.T) {
16-
sourceTypeEmptyError := "Invalid value: null"
16+
sourceTypeEmptyErrors := []string{"Invalid value: \"null\"", "Invalid value: null"}
1717
sourceTypeMismatchError := "spec.source.sourceType: Unsupported value"
1818
sourceConfigInvalidError := "spec.source: Invalid value"
1919
// unionField represents the required Catalog or (future) Bundle field required by SourceConfig
2020
testCases := []struct {
2121
name string
2222
sourceType string
2323
unionField string
24-
errMsg string
24+
errMsgs []string
2525
}{
26-
{"sourceType is null", "", "Catalog", sourceTypeEmptyError},
27-
{"sourceType is invalid", "Invalid", "Catalog", sourceTypeMismatchError},
28-
{"catalog field does not exist", "Catalog", "", sourceConfigInvalidError},
29-
{"sourceConfig has required fields", "Catalog", "Catalog", ""},
26+
{"sourceType is null", "", "Catalog", sourceTypeEmptyErrors},
27+
{"sourceType is invalid", "Invalid", "Catalog", []string{sourceTypeMismatchError}},
28+
{"catalog field does not exist", "Catalog", "", []string{sourceConfigInvalidError}},
29+
{"sourceConfig has required fields", "Catalog", "Catalog", nil},
3030
}
3131

3232
t.Parallel()
@@ -62,12 +62,20 @@ func TestClusterExtensionSourceConfig(t *testing.T) {
6262
}))
6363
}
6464

65-
if tc.errMsg == "" {
65+
if len(tc.errMsgs) == 0 {
6666
require.NoError(t, err, "unexpected error for sourceType %q: %w", tc.sourceType, err)
67-
} else {
68-
require.Error(t, err)
69-
require.Contains(t, err.Error(), tc.errMsg)
67+
return
68+
}
69+
70+
require.Error(t, err)
71+
matched := false
72+
for _, msg := range tc.errMsgs {
73+
if strings.Contains(err.Error(), msg) {
74+
matched = true
75+
break
76+
}
7077
}
78+
require.True(t, matched, "expected one of %v in error %q", tc.errMsgs, err)
7179
})
7280
}
7381
}

0 commit comments

Comments
 (0)