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

cmd/operator-sdk/scorecard: change source of truth for bundle metadata #3450

Merged
merged 1 commit into from
Jul 20, 2020

Conversation

estroz
Copy link
Member

@estroz estroz commented Jul 16, 2020

Description of the change:

  • cmd/operator-sdk/scorecard: always use on-disk metadata
  • images/scorecard-test/cmd/test: look up metadata using FindBundleMetadata
  • internal/registry: rewrite metadata lookup functions as FindBundleMetadata, which recursively searches for a file matching the annotations.yaml spec then returns that metadata and its path
  • internal/scorecard: remove 'labels' directory functionality, which is no longer needed now that metadata is always looked up recursively

Motivation for the change: This commit reverts some changes made in #3062, which use image labels as the source of truth for bundle metadata if a bundle image is supplied to scorecard. The SDK should consider the metadata directory to contain sources of truth for bundle metadata; image labels are informative only.

Checklist

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

@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 16, 2020
@estroz estroz force-pushed the refactor/scorecard-labels branch 2 times, most recently from 3931825 to 671ebcf Compare July 17, 2020 18:04
@estroz estroz changed the title [WIP] scorecard: change source of truth for bundle metadata cmd/operator-sdk/scorecard: change source of truth for bundle metadata Jul 17, 2020
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 17, 2020
@estroz
Copy link
Member Author

estroz commented Jul 17, 2020

labels, err := reg.Labels(ctx, registryimage.SimpleReference(image))
if err != nil {
return nil, fmt.Errorf("error reading image %s labels: %v", image, err)
func findBundleMetadata(fs afero.Fs, bundleRoot string) (Labels, string, error) {
Copy link
Member Author

Choose a reason for hiding this comment

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

This function in particular needs the most scrutiny.

@estroz estroz force-pushed the refactor/scorecard-labels branch from d798aa0 to 39aa250 Compare July 17, 2020 19:10
Copy link
Contributor

@jmccormick2001 jmccormick2001 left a comment

Choose a reason for hiding this comment

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

lgtm the only nit is the error message in the case of a user type-o in the annotations.yaml to cause it to not unmarshal, but that could be improved in a follow-up PR.

@estroz estroz force-pushed the refactor/scorecard-labels branch from 39aa250 to fb9dda6 Compare July 17, 2020 22:10
@estroz
Copy link
Member Author

estroz commented Jul 17, 2020

@jmccormick2001 now the error from readAnnotations is logged instead of ignored, so if the user sets --verbose they'll see the specific error.

@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 18, 2020
@estroz estroz force-pushed the refactor/scorecard-labels branch from fb9dda6 to 25b01a5 Compare July 20, 2020 17:22
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 20, 2020
…ch use image labels

as the source of truth for bundle metadata if a bundle image is supplied
to `scorecard`. The SDK should consider the metadata directory to contain
sources of truth for bundle metadata; image labels are informative only.

cmd/operator-sdk/scorecard: always use on-disk metadata

images/scorecard-test/cmd/test: look up metadata using FindBundleMetadata

internal/registry: rewrite metadata lookup functions as FindBundleMetadata,
which recursively searches for a file matching the annotations.yaml spec
then returns that metadata and its path

internal/scorecard: remove 'labels' directory functionality, which is no
longer needed now that metadata is always looked up recursively
@estroz estroz force-pushed the refactor/scorecard-labels branch from 25b01a5 to 5d443a1 Compare July 20, 2020 17:26
@estroz estroz merged commit ebe6361 into operator-framework:master Jul 20, 2020
@estroz estroz deleted the refactor/scorecard-labels branch July 20, 2020 19:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants