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.

Sign up for GitHub

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

Jump to bottom

Bug 1781101: fix(dev-catalog): reject the internal objects when creating from CSV #3518

Merged
merged 2 commits into from Dec 6, 2019
Merged

Bug 1781101: fix(dev-catalog): reject the internal objects when creating from CSV #3518

merged 2 commits into from Dec 6, 2019

Conversation

abhinandan13jan
Copy link
Contributor

@abhinandan13jan abhinandan13jan commented Nov 21, 2019 •
edited

Addresses https://jira.coreos.com/browse/ODC-2263
P.S: try-catch is used as an annotation is a free string and can have any structure
Test cases:

Test 1: CSV with annotation and internal CRD present in array
Screenshot from 2019-12-03 19-00-47
Screenshot from 2019-12-03 19-00-06

Test 2: CSV with annotation and internal CRD not listed in array
Screenshot from 2019-12-03 19-01-45
Screenshot from 2019-12-03 19-02-04

Test 3: CSV with annotation and only internal CRD
Screenshot from 2019-12-03 19-03-40
Screenshot from 2019-12-03 19-04-03

Test 4: CSV without the required annotation at all
Screenshot from 2019-12-03 19-05-22
Screenshot from 2019-12-03 19-05-40

@openshift-ci-robot openshift-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. component/core Related to console core functionality component/olm Related to OLM labels Nov 21, 2019
@abhinandan13jan abhinandan13jan changed the title fix(dev-catalog): reject the internal objects when creating from CSV [WIP]fix(dev-catalog): reject the internal objects when creating from CSV Nov 21, 2019
@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 Nov 21, 2019
@christianvogt
Copy link
Contributor

@abhinandan13jan is this ready for review now?

@abhinandan13jan abhinandan13jan changed the title [WIP]fix(dev-catalog): reject the internal objects when creating from CSV fix(dev-catalog): reject the internal objects when creating from CSV Nov 22, 2019
@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 Nov 22, 2019
@abhinandan13jan abhinandan13jan changed the title fix(dev-catalog): reject the internal objects when creating from CSV [Wfix(dev-catalog): reject the internal objects when creating from CSV Nov 22, 2019
@abhinandan13jan abhinandan13jan changed the title [Wfix(dev-catalog): reject the internal objects when creating from CSV [WIP]fix(dev-catalog): reject the internal objects when creating from CSV Nov 22, 2019
@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 Nov 22, 2019
@abhinandan13jan abhinandan13jan changed the title [WIP]fix(dev-catalog): reject the internal objects when creating from CSV fix(dev-catalog): reject the internal objects when creating from CSV Nov 22, 2019
@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 Nov 22, 2019
@abhinandan13jan
Copy link
Contributor Author

/test analyze
/test e2e-gcp-console

@abhinandan13jan
Copy link
Contributor Author

/test analyze

@abhinandan13jan
Copy link
Contributor Author

@christianvogt yes

@karthikjeeyar
Copy link
Contributor

Not able to verify the changes locally, looks like we are waiting for backend changes to be made.
Codewise it looks to me.
/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 29, 2019
@abhinandan13jan abhinandan13jan mentioned this pull request Dec 2, 2019
Merged
@christianvogt
Copy link
Contributor

Annotations are strings and your code assumes it found JSON:
image

@christianvogt
Copy link
Contributor

/lgtm cancel

@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Dec 2, 2019
TheRealJon
TheRealJon requested changes Dec 2, 2019
View reviewed changes
Copy link
Member

@TheRealJon TheRealJon left a comment

Choose a reason for hiding this comment

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

See comments. Runtime error. Annotation is being treated as a JS array, but is a string.

frontend/packages/operator-lifecycle-manager/src/dev-catalog.ts Outdated Show resolved Hide resolved
frontend/packages/operator-lifecycle-manager/src/dev-catalog.ts Outdated Show resolved Hide resolved
frontend/packages/operator-lifecycle-manager/src/dev-catalog.ts Outdated Show resolved Hide resolved
@abhinandan13jan @TheRealJon
Update frontend/packages/operator-lifecycle-manager/src/dev-catalog.ts
d2dcbf0 
Co-Authored-By: Jon Jackson <jonjacks@redhat.com>
@abhinandan13jan
Copy link
Contributor Author

@TheRealJon Committed the latest suggestion 😄

TheRealJon
TheRealJon approved these changes Dec 6, 2019
View reviewed changes
Copy link
Member

@TheRealJon TheRealJon left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Dec 6, 2019
@christianvogt
Copy link
Contributor

/approve

@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abhinandan13jan, christianvogt, karthikjeeyar, TheRealJon

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

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 6, 2019
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

10 similar comments
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-merge-robot openshift-merge-robot merged commit 9ef2a67 into openshift:master Dec 6, 2019
@abhinandan13jan abhinandan13jan changed the title fix(dev-catalog): reject the internal objects when creating from CSV Bug 1781101 fix(dev-catalog): reject the internal objects when creating from CSV Dec 9, 2019
@spadgett spadgett added this to the v4.4 milestone Dec 9, 2019
@christianvogt
Copy link
Contributor

/retitle Bug 1781101: fix(dev-catalog): reject the internal objects when creating from CSV

@openshift-ci-robot openshift-ci-robot changed the title Bug 1781101 fix(dev-catalog): reject the internal objects when creating from CSV Bug 1781101: fix(dev-catalog): reject the internal objects when creating from CSV Dec 9, 2019
@openshift-ci-robot
Copy link
Contributor

@abhinandan13jan: All pull requests linked via external trackers have merged. Bugzilla bug 1781101 has been moved to the MODIFIED state.

In response to this:

Bug 1781101: fix(dev-catalog): reject the internal objects when creating from CSV

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.

@christianvogt
Copy link
Contributor

/cherrypick release-4.3

@openshift-cherrypick-robot openshift-cherrypick-robot mentioned this pull request Dec 9, 2019
Merged
@openshift-cherrypick-robot
Copy link

@christianvogt: new pull request created: #3729

In response to this:

/cherrypick release-4.3

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.

@spadgett spadgett mentioned this pull request Jan 2, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@spadgett spadgett spadgett left review comments

@christianvogt christianvogt christianvogt left review comments

@TheRealJon TheRealJon TheRealJon approved these changes

Assignees

@karthikjeeyar karthikjeeyar

@christianvogt christianvogt

@TheRealJon TheRealJon

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. component/core Related to console core functionality component/olm Related to OLM lgtm Indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
v4.4
Development

Successfully merging this pull request may close these issues.

None yet
9 participants
@abhinandan13jan @christianvogt @karthikjeeyar @openshift-ci-robot @openshift-bot @openshift-cherrypick-robot @spadgett @TheRealJon @openshift-merge-robot