Skip to content
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

🐛 fix ClusterServiceVersion CRD Generation #324

Conversation

perdasilva
Copy link
Contributor

@perdasilva perdasilva commented May 22, 2024

New API updates is tripping up more validations against default values in the Deployment stanza of the CSV. This PR adds a couple of yq replacement queries to add default values to those fields.

seems the crds/zz_defs.go was also out of sync - regeneration seems to have fixed it

Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com>
@openshift-ci openshift-ci bot requested review from jmrodri and joelanford May 22, 2024 08:32
@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 22, 2024
Copy link

codecov bot commented May 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 39.43%. Comparing base (7d638a0) to head (eb2af81).

Current head eb2af81 differs from pull request most recent head 78fb217

Please upload reports for the commit 78fb217 to get more accurate results.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #324   +/-   ##
=======================================
  Coverage   39.43%   39.43%           
=======================================
  Files          56       56           
  Lines        4516     4516           
=======================================
  Hits         1781     1781           
  Misses       2581     2581           
  Partials      154      154           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@m1kola
Copy link
Member

m1kola commented May 22, 2024

It seems like crds/zz_defs.go needs make format

@perdasilva
Copy link
Contributor Author

I think we can override the verify here. It seems like there's been a change in the go doc for json.RawMessage:

// RawMessage is a raw encoded JSON value.
// It implements [Marshaler] and [Unmarshaler] and can
// be used to delay JSON decoding or precompute a JSON encoding.
type RawMessage []byte

https://github.com/golang/go/blob/release-branch.go1.22/src/encoding/json/stream.go#L255

This is changing the field description and causing verify to fail

Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com>
@perdasilva perdasilva force-pushed the perdasilva/bugfix/crd-generation branch from eb2af81 to 78fb217 Compare May 22, 2024 09:03
@perdasilva
Copy link
Contributor Author

It seems like crds/zz_defs.go needs make format

Thank you! I was chasing a different issue. Let's see if verify clears it (it will fail for the reasons I stated above in olm)...

Copy link

openshift-ci bot commented May 22, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: m1kola, perdasilva

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@m1kola
Copy link
Member

m1kola commented May 22, 2024

/hold

Want to check one thing before we merge

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 22, 2024
@m1kola
Copy link
Member

m1kola commented May 22, 2024

/unhold

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 22, 2024
@perdasilva
Copy link
Contributor Author

/hold - let's see if the kind job in the other PR fails first - then merge, and see if it succeeds =D

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 22, 2024
@perdasilva
Copy link
Contributor Author

/unhold

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 22, 2024
@perdasilva perdasilva merged commit 104f4b7 into operator-framework:master May 22, 2024
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants