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

Don't request instances and bindings if the user doesn't have watch rights #1769

Merged
merged 1 commit into from
Jun 26, 2017

Conversation

jwforres
Copy link
Member

No description provided.

@jwforres
Copy link
Member Author

FYI @spadgett viewers won't be able to see instances/bindings for tech preview because of the concerns around params containing secret info

Copy link
Contributor

@benjaminapetersen benjaminapetersen left a comment

Choose a reason for hiding this comment

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

LGTM, though does the /dist seem to have more changes than it should?

@@ -1349,7 +1349,9 @@ function OverviewController($scope,
updateQuotaWarnings();
}, {poll: true, pollInterval: DEFAULT_POLL_INTERVAL}));

if (SERVICE_CATALOG_ENABLED) {
var canI = $filter('canI');
// The canI check on watch should be temporary until we have a different solution for handling secret parameters
Copy link
Contributor

Choose a reason for hiding this comment

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

Curious (my own ignorance) how changing what we do for handling secret parameters will help with this watch on instances? Not saying the comment needs more necessarily, I just don't know that I follow.

Copy link
Member Author

Choose a reason for hiding this comment

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

We want instances to be watchable by project viewers, once they are then this check isn't really necessary, we dont have this check for any of the other watches today. We can't make them watchable (or listable) by project viewers yet because of the way parameters are are currently stored on instances and bindings. Viewers today should not be allowed to see potentially confidential information.

Copy link
Contributor

Choose a reason for hiding this comment

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

Awesome, thx!

@jwforres
Copy link
Member Author

travis passed, the dist is correct, the local variable minification in the overview code changed because i added a new variable

[merge]

@openshift-bot
Copy link

Evaluated for origin web console merge up to 85b5d7f

@openshift-bot
Copy link

openshift-bot commented Jun 26, 2017

Origin Web Console Merge Results: SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin_web_console/1600/) (Base Commit: 26d468a) (PR Branch Commit: 85b5d7f)

@openshift-bot openshift-bot merged commit c595098 into openshift:master Jun 26, 2017
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