Skip to content

Commit 1e25545

Browse files
authored
Merge branch 'master' into semver-candidate-fix
2 parents a3e5521 + fceaf8a commit 1e25545

File tree

18 files changed

+635
-108
lines changed

18 files changed

+635
-108
lines changed

staging/operator-lifecycle-manager/pkg/controller/operators/olm/operator.go

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
1212
corev1 "k8s.io/api/core/v1"
1313
rbacv1 "k8s.io/api/rbac/v1"
14+
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
1415
extinf "k8s.io/apiextensions-apiserver/pkg/client/informers/externalversions"
1516
apierrors "k8s.io/apimachinery/pkg/api/errors"
1617
"k8s.io/apimachinery/pkg/api/meta"
@@ -1121,6 +1122,46 @@ func (a *Operator) handleClusterServiceVersionDeletion(obj interface{}) {
11211122
logger.WithError(err).Warnf("failed to requeue gc event: %v", webhook)
11221123
}
11231124
}
1125+
1126+
// Conversion webhooks are defined within a CRD.
1127+
// In an effort to prevent customer dataloss, OLM does not delete CRDs associated with a CSV when it is deleted.
1128+
// Deleting a CSV that introduced a conversion webhook removes the deployment that serviced the conversion webhook calls.
1129+
// If a conversion webhook is defined and the service isn't available, all requests against the CR associated with the CRD will fail.
1130+
// This ultimately breaks kubernetes garbage collection and prevents OLM from reinstalling the CSV as CR validation against the new CRD's
1131+
// openapiv3 schema fails.
1132+
// As such, when a CSV is deleted OLM will check if it is being replaced. If the CSV is not being replaced, OLM will remove the conversion
1133+
// webhook from the CRD definition.
1134+
csvs, err := a.lister.OperatorsV1alpha1().ClusterServiceVersionLister().ClusterServiceVersions(clusterServiceVersion.GetNamespace()).List(labels.Everything())
1135+
if err != nil {
1136+
logger.Errorf("error listing csvs: %v\n", err)
1137+
}
1138+
for _, csv := range csvs {
1139+
if csv.Spec.Replaces == clusterServiceVersion.GetName() {
1140+
return
1141+
}
1142+
}
1143+
1144+
for _, desc := range clusterServiceVersion.Spec.WebhookDefinitions {
1145+
if desc.Type != v1alpha1.ConversionWebhook || len(desc.ConversionCRDs) == 0 {
1146+
continue
1147+
}
1148+
1149+
for i, crdName := range desc.ConversionCRDs {
1150+
crd, err := a.lister.APIExtensionsV1().CustomResourceDefinitionLister().Get(crdName)
1151+
if err != nil {
1152+
logger.Errorf("error getting CRD %v which was defined in CSVs spec.WebhookDefinition[%d]: %v\n", crdName, i, err)
1153+
continue
1154+
}
1155+
1156+
copy := crd.DeepCopy()
1157+
copy.Spec.Conversion.Strategy = apiextensionsv1.NoneConverter
1158+
copy.Spec.Conversion.Webhook = nil
1159+
1160+
if _, err = a.opClient.ApiextensionsInterface().ApiextensionsV1().CustomResourceDefinitions().Update(context.TODO(), copy, metav1.UpdateOptions{}); err != nil {
1161+
logger.Errorf("error updating conversion strategy for CRD %v: %v\n", crdName, err)
1162+
}
1163+
}
1164+
}
11241165
}
11251166

11261167
func (a *Operator) removeDanglingChildCSVs(csv *v1alpha1.ClusterServiceVersion) error {

staging/operator-lifecycle-manager/test/e2e/csv_e2e_test.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4206,6 +4206,9 @@ func findLastEvent(events *corev1.EventList) (event corev1.Event) {
42064206
func buildCSVCleanupFunc(c operatorclient.ClientInterface, crc versioned.Interface, csv operatorsv1alpha1.ClusterServiceVersion, namespace string, deleteCRDs, deleteAPIServices bool) cleanupFunc {
42074207
return func() {
42084208
err := crc.OperatorsV1alpha1().ClusterServiceVersions(namespace).Delete(context.TODO(), csv.GetName(), metav1.DeleteOptions{})
4209+
if err != nil && apierrors.IsNotFound(err) {
4210+
err = nil
4211+
}
42094212
Expect(err).ShouldNot(HaveOccurred())
42104213

42114214
if deleteCRDs {

staging/operator-lifecycle-manager/test/e2e/webhook_e2e_test.go

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -639,6 +639,7 @@ var _ = Describe("CSVs with a Webhook", func() {
639639
}).Should(Equal(2))
640640
})
641641
When("Installed from a catalog Source", func() {
642+
const csvName = "webhook-operator.v0.0.1"
642643
var cleanupCSV cleanupFunc
643644
var cleanupCatSrc cleanupFunc
644645
var cleanupSubscription cleanupFunc
@@ -687,7 +688,7 @@ var _ = Describe("CSVs with a Webhook", func() {
687688
defer cleanupSubscription()
688689

689690
// Wait for webhook-operator v2 csv to succeed
690-
csv, err := awaitCSV(crc, source.GetNamespace(), "webhook-operator.v0.0.1", csvSucceededChecker)
691+
csv, err := awaitCSV(crc, source.GetNamespace(), csvName, csvSucceededChecker)
691692
require.NoError(GinkgoT(), err)
692693

693694
cleanupCSV = buildCSVCleanupFunc(c, crc, *csv, source.GetNamespace(), true, true)
@@ -775,6 +776,25 @@ var _ = Describe("CSVs with a Webhook", func() {
775776
require.True(GinkgoT(), ok, "Unable to get spec.conversion.valid of v2 object")
776777
require.True(GinkgoT(), v2SpecConversionMutate)
777778
require.True(GinkgoT(), v2SpecConversionValid)
779+
780+
// Check that conversion strategies are disabled after uninstalling the operator.
781+
err = crc.OperatorsV1alpha1().ClusterServiceVersions(generatedNamespace.GetName()).Delete(context.TODO(), csvName, metav1.DeleteOptions{})
782+
require.NoError(GinkgoT(), err)
783+
784+
Eventually(func() error {
785+
crd, err := c.ApiextensionsInterface().ApiextensionsV1().CustomResourceDefinitions().Get(context.TODO(), "webhooktests.webhook.operators.coreos.io", metav1.GetOptions{})
786+
if err != nil {
787+
return err
788+
}
789+
790+
if crd.Spec.Conversion.Strategy != apiextensionsv1.NoneConverter {
791+
return fmt.Errorf("conversion strategy is not NoneConverter")
792+
}
793+
if crd.Spec.Conversion.Webhook != nil {
794+
return fmt.Errorf("webhook is not nil")
795+
}
796+
return nil
797+
}).Should(Succeed())
778798
})
779799
})
780800
When("WebhookDescription has conversionCRDs field", func() {

staging/operator-registry/alpha/declcfg/write.go

Lines changed: 137 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -8,67 +8,90 @@ import (
88
"sort"
99
"strings"
1010

11+
"github.com/blang/semver/v4"
12+
"github.com/operator-framework/operator-registry/alpha/property"
1113
"k8s.io/apimachinery/pkg/util/sets"
1214
"sigs.k8s.io/yaml"
1315
)
1416

1517
// writes out the channel edges of the declarative config graph in a mermaid format capable of being pasted into
1618
// mermaid renderers like github, mermaid.live, etc.
1719
// output is sorted lexicographically by package name, and then by channel name
20+
// if provided, minEdgeName will be used as the lower bound for edges in the output graph
1821
//
1922
// NB: Output has wrapper comments stating the skipRange edge caveat in HTML comment format, which cannot be parsed by mermaid renderers.
20-
// This is deliberate, and intended as an explicit acknowledgement of the limitations, instead of requiring the user to notice the missing edges upon inspection.
23+
//
24+
// This is deliberate, and intended as an explicit acknowledgement of the limitations, instead of requiring the user to notice the missing edges upon inspection.
2125
//
2226
// Example output:
2327
// <!-- PLEASE NOTE: skipRange edges are not currently displayed -->
2428
// graph LR
25-
// %% package "neuvector-certified-operator-rhmp"
26-
// subgraph "neuvector-certified-operator-rhmp"
27-
// %% channel "beta"
28-
// subgraph neuvector-certified-operator-rhmp-beta["beta"]
29-
// neuvector-certified-operator-rhmp-beta-neuvector-operator.v1.2.8["neuvector-operator.v1.2.8"]
30-
// neuvector-certified-operator-rhmp-beta-neuvector-operator.v1.2.9["neuvector-operator.v1.2.9"]
31-
// neuvector-certified-operator-rhmp-beta-neuvector-operator.v1.3.0["neuvector-operator.v1.3.0"]
32-
// neuvector-certified-operator-rhmp-beta-neuvector-operator.v1.3.0["neuvector-operator.v1.3.0"]-- replaces --> neuvector-certified-operator-rhmp-beta-neuvector-operator.v1.2.8["neuvector-operator.v1.2.8"]
33-
// neuvector-certified-operator-rhmp-beta-neuvector-operator.v1.3.0["neuvector-operator.v1.3.0"]-- skips --> neuvector-certified-operator-rhmp-beta-neuvector-operator.v1.2.9["neuvector-operator.v1.2.9"]
34-
// end
35-
// end
29+
//
30+
// %% package "neuvector-certified-operator-rhmp"
31+
// subgraph "neuvector-certified-operator-rhmp"
32+
// %% channel "beta"
33+
// subgraph neuvector-certified-operator-rhmp-beta["beta"]
34+
// neuvector-certified-operator-rhmp-beta-neuvector-operator.v1.2.8["neuvector-operator.v1.2.8"]
35+
// neuvector-certified-operator-rhmp-beta-neuvector-operator.v1.2.9["neuvector-operator.v1.2.9"]
36+
// neuvector-certified-operator-rhmp-beta-neuvector-operator.v1.3.0["neuvector-operator.v1.3.0"]
37+
// neuvector-certified-operator-rhmp-beta-neuvector-operator.v1.3.0["neuvector-operator.v1.3.0"]-- replaces --> neuvector-certified-operator-rhmp-beta-neuvector-operator.v1.2.8["neuvector-operator.v1.2.8"]
38+
// neuvector-certified-operator-rhmp-beta-neuvector-operator.v1.3.0["neuvector-operator.v1.3.0"]-- skips --> neuvector-certified-operator-rhmp-beta-neuvector-operator.v1.2.9["neuvector-operator.v1.2.9"]
39+
// end
40+
// end
41+
//
3642
// end
3743
// <!-- PLEASE NOTE: skipRange edges are not currently displayed -->
38-
func WriteMermaidChannels(cfg DeclarativeConfig, out io.Writer) error {
44+
func WriteMermaidChannels(cfg DeclarativeConfig, out io.Writer, minEdgeName string) error {
3945
pkgs := map[string]*strings.Builder{}
4046

4147
sort.Slice(cfg.Channels, func(i, j int) bool {
4248
return cfg.Channels[i].Name < cfg.Channels[j].Name
4349
})
4450

45-
for _, c := range cfg.Channels {
46-
pkgBuilder, ok := pkgs[c.Package]
47-
if !ok {
48-
pkgBuilder = &strings.Builder{}
49-
pkgs[c.Package] = pkgBuilder
51+
versionMap, err := getBundleVersions(&cfg)
52+
if err != nil {
53+
return err
54+
}
55+
56+
if _, ok := versionMap[minEdgeName]; !ok {
57+
if minEdgeName != "" {
58+
return fmt.Errorf("unknown minimum edge name: %q", minEdgeName)
5059
}
51-
channelID := fmt.Sprintf("%s-%s", c.Package, c.Name)
52-
pkgBuilder.WriteString(fmt.Sprintf(" %%%% channel %q\n", c.Name))
53-
pkgBuilder.WriteString(fmt.Sprintf(" subgraph %s[%q]\n", channelID, c.Name))
54-
55-
for _, ce := range c.Entries {
56-
entryId := fmt.Sprintf("%s-%s", channelID, ce.Name)
57-
pkgBuilder.WriteString(fmt.Sprintf(" %s[%q]\n", entryId, ce.Name))
58-
59-
// no support for SkipRange yet
60-
if len(ce.Replaces) > 0 {
61-
replacesId := fmt.Sprintf("%s-%s", channelID, ce.Replaces)
62-
pkgBuilder.WriteString(fmt.Sprintf(" %s[%q]-- %s --> %s[%q]\n", entryId, ce.Name, "replaces", replacesId, ce.Replaces))
60+
}
61+
62+
for _, c := range cfg.Channels {
63+
filteredChannel := filterChannel(&c, versionMap, minEdgeName)
64+
if filteredChannel != nil {
65+
pkgBuilder, ok := pkgs[c.Package]
66+
if !ok {
67+
pkgBuilder = &strings.Builder{}
68+
pkgs[c.Package] = pkgBuilder
6369
}
64-
if len(ce.Skips) > 0 {
65-
for _, s := range ce.Skips {
66-
skipsId := fmt.Sprintf("%s-%s", channelID, s)
67-
pkgBuilder.WriteString(fmt.Sprintf(" %s[%q]-- %s --> %s[%q]\n", entryId, ce.Name, "skips", skipsId, s))
70+
71+
channelID := fmt.Sprintf("%s-%s", filteredChannel.Package, filteredChannel.Name)
72+
pkgBuilder.WriteString(fmt.Sprintf(" %%%% channel %q\n", filteredChannel.Name))
73+
pkgBuilder.WriteString(fmt.Sprintf(" subgraph %s[%q]\n", channelID, filteredChannel.Name))
74+
75+
for _, ce := range filteredChannel.Entries {
76+
if versionMap[ce.Name].GE(versionMap[minEdgeName]) {
77+
entryId := fmt.Sprintf("%s-%s", channelID, ce.Name)
78+
pkgBuilder.WriteString(fmt.Sprintf(" %s[%q]\n", entryId, ce.Name))
79+
80+
// no support for SkipRange yet
81+
if len(ce.Replaces) > 0 {
82+
replacesId := fmt.Sprintf("%s-%s", channelID, ce.Replaces)
83+
pkgBuilder.WriteString(fmt.Sprintf(" %s[%q]-- %s --> %s[%q]\n", entryId, ce.Name, "replaces", replacesId, ce.Replaces))
84+
}
85+
if len(ce.Skips) > 0 {
86+
for _, s := range ce.Skips {
87+
skipsId := fmt.Sprintf("%s-%s", channelID, s)
88+
pkgBuilder.WriteString(fmt.Sprintf(" %s[%q]-- %s --> %s[%q]\n", entryId, ce.Name, "skips", skipsId, s))
89+
}
90+
}
6891
}
6992
}
93+
pkgBuilder.WriteString(" end\n")
7094
}
71-
pkgBuilder.WriteString(" end\n")
7295
}
7396

7497
out.Write([]byte("<!-- PLEASE NOTE: skipRange edges are not currently displayed -->\n"))
@@ -91,6 +114,85 @@ func WriteMermaidChannels(cfg DeclarativeConfig, out io.Writer) error {
91114
return nil
92115
}
93116

117+
// filters the channel edges to include only those which are greater-than-or-equal to the edge named by startVersion
118+
// returns a nil channel if all edges are filtered out
119+
func filterChannel(c *Channel, versionMap map[string]semver.Version, minEdgeName string) *Channel {
120+
// short-circuit if no specified startVersion
121+
if minEdgeName == "" {
122+
return c
123+
}
124+
// convert the edge name to the version so we don't have to duplicate the lookup
125+
minVersion := versionMap[minEdgeName]
126+
127+
out := &Channel{Name: c.Name, Package: c.Package, Properties: c.Properties, Entries: []ChannelEntry{}}
128+
for _, ce := range c.Entries {
129+
filteredCe := ChannelEntry{Name: ce.Name}
130+
// short-circuit to take the edge name (but no references to earlier versions)
131+
if ce.Name == minEdgeName {
132+
out.Entries = append(out.Entries, filteredCe)
133+
continue
134+
}
135+
// if len(ce.SkipRange) > 0 {
136+
// }
137+
if len(ce.Replaces) > 0 {
138+
if versionMap[ce.Replaces].GTE(minVersion) {
139+
filteredCe.Replaces = ce.Replaces
140+
}
141+
}
142+
if len(ce.Skips) > 0 {
143+
filteredSkips := []string{}
144+
for _, s := range ce.Skips {
145+
if versionMap[s].GTE(minVersion) {
146+
filteredSkips = append(filteredSkips, s)
147+
}
148+
}
149+
if len(filteredSkips) > 0 {
150+
filteredCe.Skips = filteredSkips
151+
}
152+
}
153+
if len(filteredCe.Replaces) > 0 || len(filteredCe.Skips) > 0 {
154+
out.Entries = append(out.Entries, filteredCe)
155+
}
156+
}
157+
158+
if len(out.Entries) > 0 {
159+
return out
160+
} else {
161+
return nil
162+
}
163+
}
164+
165+
func parseVersionProperty(b *Bundle) (*semver.Version, error) {
166+
props, err := property.Parse(b.Properties)
167+
if err != nil {
168+
return nil, fmt.Errorf("parse properties for bundle %q: %v", b.Name, err)
169+
}
170+
if len(props.Packages) != 1 {
171+
return nil, fmt.Errorf("bundle %q has multiple %q properties, expected exactly 1", b.Name, property.TypePackage)
172+
}
173+
v, err := semver.Parse(props.Packages[0].Version)
174+
if err != nil {
175+
return nil, fmt.Errorf("bundle %q has invalid version %q: %v", b.Name, props.Packages[0].Version, err)
176+
}
177+
178+
return &v, nil
179+
}
180+
181+
func getBundleVersions(cfg *DeclarativeConfig) (map[string]semver.Version, error) {
182+
entries := make(map[string]semver.Version)
183+
for index := range cfg.Bundles {
184+
if _, ok := entries[cfg.Bundles[index].Name]; !ok {
185+
ver, err := parseVersionProperty(&cfg.Bundles[index])
186+
if err != nil {
187+
return entries, err
188+
}
189+
entries[cfg.Bundles[index].Name] = *ver
190+
}
191+
}
192+
193+
return entries, nil
194+
}
195+
94196
func WriteJSON(cfg DeclarativeConfig, w io.Writer) error {
95197
enc := json.NewEncoder(w)
96198
enc.SetIndent("", " ")

staging/operator-registry/alpha/declcfg/write_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -513,10 +513,11 @@ graph LR
513513
`,
514514
},
515515
}
516+
startVersion := ""
516517
for _, s := range specs {
517518
t.Run(s.name, func(t *testing.T) {
518519
var buf bytes.Buffer
519-
err := WriteMermaidChannels(s.cfg, &buf)
520+
err := WriteMermaidChannels(s.cfg, &buf, startVersion)
520521
require.NoError(t, err)
521522
require.Equal(t, s.expected, buf.String())
522523
})

staging/operator-registry/alpha/veneer/semver/README.md

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,9 +56,10 @@ In this example, `Candidate` has the entire version range of bundles, `Fast` ha
5656
```
5757
% ./bin/opm alpha render-veneer semver -h
5858
Generate a file-based catalog from a single 'semver veneer' file
59+
When FILE is '-' or not provided, the veneer is read from standard input
5960

6061
Usage:
61-
opm alpha render-veneer semver <filename> [flags]
62+
opm alpha render-veneer semver [FILE] [flags]
6263

6364
Flags:
6465
-h, --help help for semver
@@ -68,6 +69,22 @@ Global Flags:
6869
--skip-tls-verify skip TLS certificate verification for container image registries while pulling bundles
6970
--use-http use plain HTTP for container image registries while pulling bundles
7071
```
72+
73+
Example command usage:
74+
```
75+
# Example with file argument passed in
76+
opm alpha render-veneer semver infile.semver.veneer.yaml
77+
78+
# Example with no file argument passed in
79+
opm alpha render-veneer semver -o yaml < infile.semver.veneer.yaml > outfile.yaml
80+
81+
# Example with "-" as the file argument passed in
82+
cat infile.semver.veneer.yaml | opm alpha render-veneer semver -o mermaid -
83+
```
84+
Note that if the command is called without a file argument and nothing passed in on standard input,
85+
the command will hang indefinitely. Either a file argument or file information passed
86+
in on standard input is required by the command.
87+
7188
With the veneer attribute `GenerateMajorChannels: true` resulting major channels from the command are (skipping the rendered bundle image output):
7289
```yaml
7390
---

staging/operator-registry/cmd/opm/alpha/cmd.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"github.com/operator-framework/operator-registry/cmd/opm/alpha/bundle"
77
"github.com/operator-framework/operator-registry/cmd/opm/alpha/diff"
88
"github.com/operator-framework/operator-registry/cmd/opm/alpha/list"
9+
rendergraph "github.com/operator-framework/operator-registry/cmd/opm/alpha/render-graph"
910
"github.com/operator-framework/operator-registry/cmd/opm/alpha/veneer"
1011
)
1112

@@ -20,6 +21,7 @@ func NewCmd() *cobra.Command {
2021
runCmd.AddCommand(
2122
bundle.NewCmd(),
2223
list.NewCmd(),
24+
rendergraph.NewCmd(),
2325
diff.NewCmd(),
2426
veneer.NewCmd(),
2527
)

0 commit comments

Comments
 (0)