🐛 combine bundle properties from csv and metadata/properties.yaml #1495
Conversation
✅ Deploy Preview for olmv1 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
| if err != nil && !errors.Is(err, fs.ErrNotExist) { | ||
| return fmt.Errorf("failed to read properties.yaml file: %w", err) | ||
| } | ||
| if len(metadataPropertiesJSON) > 0 { |
There was a problem hiding this comment.
If there are no metadataPropertiesJSON, then you can early exit this function
There was a problem hiding this comment.
That would also decrease the overall indentation of the function...
Signed-off-by: Joe Lanford <[email protected]>
joelanford
force-pushed
the
support-metadata-properties-yaml
branch
from
61cf3b3 to
1426638
Compare
| 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"}]`)) | ||
| }) | ||
| }) | ||
|
|
There was a problem hiding this comment.
ACK! We didn't get rid of Ginkgo everywhere‽‽
There was a problem hiding this comment.
Or we did, but it came back when rukpak was pulled in.
There was a problem hiding this comment.
And I was part of that... oh well...
|
It LGTM, but are we waiting until 1.0.0, or not? |
|
We can, not sure it matters either way since it is a bug fix. |
|
As long as we're taking it into consideration... |
|
Yep, good call out! |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1495 +/- ##
==========================================
+ Coverage 74.67% 74.68% +0.01%
==========================================
Files 42 42
Lines 3234 3271 +37
==========================================
+ Hits 2415 2443 +28
- Misses 646 652 +6
- Partials 173 176 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
Yeah, we can merge bugfixes before 1.0.0. |
b9294a7
When we implemented support for including CSV annotations in chart metadata, we neglected to remember that bundle properties can also come from the
metadata/properties.yamlfile in the bundle filesystem. This PR updates the registry+v1 bundle converter library to read properties frommetadata/properties.yaml(if it exists) and append any properties that are found into themetadata.annotations["olm.properties"]annotation in the CSV.Description
Reviewer Checklist