scorecard,generate-csv: consider api "group" while checking for known types #3334
Conversation
|
LGTM |
...testdata/non-standard-layout/config/crds-with-core-types/foo.example.com_deployment_crd.yaml
Show resolved
Hide resolved
There was a problem hiding this comment.
Really thank you for your contribution.
It shows ok for me.
Please just give the space in the fragment and let's see what @estroz and @jmccormick2001 think about these changes as well.
Otherwise, it is /lgtm /approve.
| @@ -359,6 +359,37 @@ var _ = Describe("Generating a ClusterServiceVersion", func() { | |||
| Expect(csv).To(Equal(upgradeCSV(newCSV, g.OperatorName, g.Version))) | |||
| }) | |||
| }) | |||
|
|
|||
| Context("generate ClusterServiceVersion", func() { | |||
There was a problem hiding this comment.
Tests for the functionality added in this PR should be in internal/generate/collector, not here.
There was a problem hiding this comment.
Currently, there are no unit tests exist specific to internal/generate/collector functionality. The clusterserviceversion_test.go also covers collector functionality, hence I add a new case to test this new change.
There was a problem hiding this comment.
Well you can add a test to internal/generate/collector so specific tests exist 😃
There was a problem hiding this comment.
@estroz Added a test for chagne in collector package.
There was a problem hiding this comment.
@avalluri thanks! Can you also remove this test, since the new functionality is being tested elsewhere?
There was a problem hiding this comment.
@estroz Thanks for your review time. I intentionally left the test in clustersevice_version.go as the test also ensures the alm-examples annotations filled properly even for CRs with core type names. I made changes to test such that it makes sure the generated alm-examples matches with found CRs.
openshift-ci-robot
added
the
needs-rebase
avalluri
force-pushed
the
fix-deployments
branch
from
8ff664a to
503357d
Compare
openshift-ci-robot
removed
the
needs-rebase
avalluri
force-pushed
the
fix-deployments
branch
2 times, most recently
from
b67b2d7 to
4e2a5ff
Compare
|
This PR is a good first step towards a (potential) larger refactor needed to support #3494. |
avalluri
force-pushed
the
fix-deployments
branch
from
4e2a5ff to
c5981a1
Compare
openshift-ci-robot
added
the
needs-rebase
avalluri
force-pushed
the
fix-deployments
branch
from
c5981a1 to
bfdab49
Compare
openshift-ci-robot
removed
the
needs-rebase
avalluri
force-pushed
the
fix-deployments
branch
from
9fbd17d to
708f6d9
Compare
… types Comparing only "kind" of the resource might lead to conflict where two resources might have same kind but in different api groups. For example an operator could provide a custom resource with name "Deployment" in it's own group. On generating csv for this operator might not consider the provided CR for filling 'alm-examples'.
avalluri
force-pushed
the
fix-deployments
branch
from
708f6d9 to
7a92389
Compare
Description of the change:
Comparing only "kind" of the resource might lead to conflict where two resources might have same kind but in different api groups.
For example an operator could provide a custom resource with name "Deployment" in it's own group. On generating csv for this operator might not consider the provided CR for filling 'alm-examples'
Motivation for the change:
Observed this issue while integrating PMEM-CSI operator which provides
DeploymentCRD inpmem-csi.intel.comAPI group.Checklist
If the pull request includes user-facing changes, extra documentation is required:
changelog/fragments(seechangelog/fragments/00-template.yaml)website/content/en/docs