Skip to content

Conversation

@estroz
Copy link
Member

@estroz estroz commented Jan 28, 2021

Description of the change:

  • internal/generatel/clusterserviceversion/bases/definitions: make the owned CRD generator package- and type-aware so multiple packages containing the same type names can be used.

Motivation for the change: currently the CSV generator indexes types in the struct-field hierarchy by type name only, which is potentially not unique between packages. This PR makes the generator package-aware.

Closes #4409
Closes #3744

/kind bug

Signed-off-by: Eric Stroczynski [email protected]

Checklist

If the pull request includes user-facing changes, extra documentation is required:

owned CRD generator package- and type-aware so multiple packages
containing the same type names can be used.

Signed-off-by: Eric Stroczynski <[email protected]>
@openshift-ci-robot openshift-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label Jan 28, 2021
@estroz estroz added this to the v1.4.0 milestone Jan 28, 2021
@estroz
Copy link
Member Author

estroz commented Feb 1, 2021

/assign @jmrodri

Copy link
Member

@jmrodri jmrodri left a comment

Choose a reason for hiding this comment

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

/lgtm

I would've preferred to see unit tests directly for the ast.go file but seems to be covered by the larger definitions_test.

}

// findPackagePathForSelExpr returns the package path corresponding to the package name used in expr if it exists in im.
func (im importIdents) findPackagePathForSelExpr(expr *ast.SelectorExpr) (pkgPath string) {
Copy link
Member

Choose a reason for hiding this comment

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

Nit: Not a fan of this method name but I don't have a better option :) so not blocking this PR.

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Feb 2, 2021
@estroz estroz merged commit 169778f into operator-framework:master Feb 2, 2021
@estroz estroz deleted the bugfix/package-scoped-csv-types branch February 2, 2021 20:50
reinvantveer pushed a commit to reinvantveer/operator-sdk that referenced this pull request Feb 4, 2021
…ework#4445)

internal/generatel/clusterserviceversion/bases/definitions: make the
owned CRD generator package- and type-aware so multiple packages
containing the same type names can be used.

Signed-off-by: Eric Stroczynski <[email protected]>
Signed-off-by: Rein van 't Veer <[email protected]>
reinvantveer pushed a commit to reinvantveer/operator-sdk that referenced this pull request Feb 4, 2021
…ework#4445)

internal/generatel/clusterserviceversion/bases/definitions: make the
owned CRD generator package- and type-aware so multiple packages
containing the same type names can be used.

Signed-off-by: Eric Stroczynski <[email protected]>
Signed-off-by: Rein van 't Veer <[email protected]>
@estroz
Copy link
Member Author

estroz commented Feb 4, 2021

/cherry-pick v1.3.x

@openshift-cherrypick-robot

@estroz: new pull request created: #4480

Details

In response to this:

/cherry-pick v1.3.x

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

reinvantveer pushed a commit to reinvantveer/operator-sdk that referenced this pull request Feb 5, 2021
…ework#4445)

internal/generatel/clusterserviceversion/bases/definitions: make the
owned CRD generator package- and type-aware so multiple packages
containing the same type names can be used.

Signed-off-by: Eric Stroczynski <[email protected]>
Signed-off-by: Rein van 't Veer <[email protected]>
reinvantveer pushed a commit to reinvantveer/operator-sdk that referenced this pull request Feb 5, 2021
…ework#4445)

internal/generatel/clusterserviceversion/bases/definitions: make the
owned CRD generator package- and type-aware so multiple packages
containing the same type names can be used.

Signed-off-by: Eric Stroczynski <[email protected]>
Signed-off-by: reinvantveer <[email protected]>
reinvantveer pushed a commit to reinvantveer/operator-sdk that referenced this pull request Feb 5, 2021
…ework#4445)

internal/generatel/clusterserviceversion/bases/definitions: make the
owned CRD generator package- and type-aware so multiple packages
containing the same type names can be used.

Signed-off-by: Eric Stroczynski <[email protected]>
Signed-off-by: Rein van 't Veer <[email protected]>
Signed-off-by: reinvantveer <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/bug Categorizes issue or PR as related to a bug. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CSV specDescriptors for type pulled from wrong GVK CSV Field Markers are not collected if they are in another go file

4 participants