Skip to content

🐛 combine bundle properties from csv and metadata/properties.yaml #1495

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 56 additions & 2 deletions internal/rukpak/convert/registryv1.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"crypto/sha256"
"encoding/json"
"errors"
"fmt"
"io/fs"
"path/filepath"
Expand All @@ -23,6 +24,7 @@ import (
"sigs.k8s.io/yaml"

"github.com/operator-framework/api/pkg/operators/v1alpha1"
"github.com/operator-framework/operator-registry/alpha/property"
registrybundle "github.com/operator-framework/operator-registry/pkg/lib/bundle"

registry "github.com/operator-framework/operator-controller/internal/rukpak/operator-registry"
Expand All @@ -43,12 +45,12 @@ func RegistryV1ToHelmChart(ctx context.Context, rv1 fs.FS, installNamespace stri
l := log.FromContext(ctx)

reg := RegistryV1{}
fileData, err := fs.ReadFile(rv1, filepath.Join("metadata", "annotations.yaml"))
annotationsFileData, err := fs.ReadFile(rv1, filepath.Join("metadata", "annotations.yaml"))
if err != nil {
return nil, err
}
annotationsFile := registry.AnnotationsFile{}
if err := yaml.Unmarshal(fileData, &annotationsFile); err != nil {
if err := yaml.Unmarshal(annotationsFileData, &annotationsFile); err != nil {
return nil, err
}
reg.PackageName = annotationsFile.Annotations.PackageName
Expand Down Expand Up @@ -101,9 +103,61 @@ func RegistryV1ToHelmChart(ctx context.Context, rv1 fs.FS, installNamespace stri
return nil, err
}

if err := copyMetadataPropertiesToCSV(&reg.CSV, rv1); err != nil {
return nil, err
}

return toChart(reg, installNamespace, watchNamespaces)
}

// copyMetadataPropertiesToCSV copies properties from `metadata/propeties.yaml` (in the filesystem fsys) into
// the CSV's `.metadata.annotations['olm.properties']` value, preserving any properties that are already
// present in the annotations.
func copyMetadataPropertiesToCSV(csv *v1alpha1.ClusterServiceVersion, fsys fs.FS) error {
var allProperties []property.Property

// First load existing properties from the CSV. We want to preserve these.
if csvPropertiesJSON, ok := csv.Annotations["olm.properties"]; ok {
var csvProperties []property.Property
if err := json.Unmarshal([]byte(csvPropertiesJSON), &csvProperties); err != nil {
return fmt.Errorf("failed to unmarshal csv.metadata.annotations['olm.properties']: %w", err)
}
allProperties = append(allProperties, csvProperties...)
}

// Next, load properties from the metadata/properties.yaml file, if it exists.
metadataPropertiesJSON, err := fs.ReadFile(fsys, filepath.Join("metadata", "properties.yaml"))
if err != nil && !errors.Is(err, fs.ErrNotExist) {
return fmt.Errorf("failed to read properties.yaml file: %w", err)
}

// If there are no properties, we can stick with whatever
// was already present in the CSV annotations.
if len(metadataPropertiesJSON) == 0 {
return nil
}

// Otherwise, we need to parse the properties.yaml file and
// append its properties into the CSV annotation.
type registryV1Properties struct {
Properties []property.Property `json:"properties"`
}

var metadataProperties registryV1Properties
if err := yaml.Unmarshal(metadataPropertiesJSON, &metadataProperties); err != nil {
return fmt.Errorf("failed to unmarshal metadata/properties.yaml: %w", err)
}
allProperties = append(allProperties, metadataProperties.Properties...)

// Lastly re-marshal all the properties back into a JSON array and update the CSV annotation
allPropertiesJSON, err := json.Marshal(allProperties)
if err != nil {
return fmt.Errorf("failed to marshal registry+v1 properties to json: %w", err)
}
csv.Annotations["olm.properties"] = string(allPropertiesJSON)
return nil
}

func toChart(in RegistryV1, installNamespace string, watchNamespaces []string) (*chart.Chart, error) {
plain, err := Convert(in, installNamespace, watchNamespaces)
if err != nil {
Expand Down
15 changes: 14 additions & 1 deletion internal/rukpak/convert/registryv1_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
package convert

import (
"context"
"fmt"
"os"
"strings"
"testing"

Expand All @@ -24,7 +26,7 @@ import (

func TestRegistryV1Converter(t *testing.T) {
RegisterFailHandler(Fail)
RunSpecs(t, "RegstryV1 suite")
RunSpecs(t, "RegistryV1 suite")
}

var _ = Describe("RegistryV1 Suite", func() {
Expand Down Expand Up @@ -418,6 +420,17 @@ var _ = Describe("RegistryV1 Suite", func() {
})
})

Context("Should read the registry+v1 bundle filesystem correctly", func() {
It("should include metadata/properties.yaml and csv.metadata.annotations['olm.properties'] in chart metadata", func() {
fsys := os.DirFS("testdata/combine-properties-bundle")
chrt, err := RegistryV1ToHelmChart(context.Background(), fsys, "", nil)
Expect(err).NotTo(HaveOccurred())
Expect(chrt).NotTo(BeNil())
Expect(chrt.Metadata).NotTo(BeNil())
Expect(chrt.Metadata.Annotations).To(HaveKeyWithValue("olm.properties", `[{"type":"from-csv-annotations-key","value":"from-csv-annotations-value"},{"type":"from-file-key","value":"from-file-value"}]`))
})
})

Comment on lines +423 to +433
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK! We didn't get rid of Ginkgo everywhere‽‽

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or we did, but it came back when rukpak was pulled in.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And I was part of that... oh well...

Context("Should enforce limitations", func() {
It("should not allow bundles with webhooks", func() {
By("creating a registry v1 bundle")
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
apiVersion: operators.operatorframework.io/v1alpha1
kind: ClusterServiceVersion
metadata:
name: test.v1.0.0
annotations:
olm.properties: '[{"type":"from-csv-annotations-key", "value":"from-csv-annotations-value"}]'
spec:
installModes:
- type: AllNamespaces
supported: true
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
annotations:
operators.operatorframework.io.bundle.mediatype.v1: registry+v1
operators.operatorframework.io.bundle.package.v1: test
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
properties:
- type: "from-file-key"
value: "from-file-value"
Loading