Skip to content

Conversation

@jcantrill
Copy link
Contributor

…ient

This bug resolves https://bugzilla.redhat.com/show_bug.cgi?id=1732585 by:

  • Modifying kibana-proxy to rely on ServiceAccount as an oauthclient so that we no longer need to keep a secret and and oauthclient in sync

@openshift-ci-robot
Copy link

@jcantrill: This pull request references Bugzilla bug 1732585, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

In response to this:

Bug 1732585: Modify kibana-proxy to rely on ServiceAccount as oauthcl…

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.

@openshift-ci-robot openshift-ci-robot added the bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. label Aug 23, 2019
@jcantrill jcantrill added the kind/bug Categorizes issue or PR as related to a bug. label Aug 23, 2019
@openshift-ci-robot openshift-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Aug 23, 2019
@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 23, 2019
"-cookie-secret-file=/secret/session-secret",
"-upstream=http://localhost:5601",
"-scope=user:info user:check-access user:list-projects",
"-scope=user:info user:check-access",
Copy link
Contributor

Choose a reason for hiding this comment

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

we don't need list-projects here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm uncertain why this is here as the Kibana proxy never needed to list projects. All of that particular logic is in the openshift-elasticsearch-plugin.

Copy link
Contributor

Choose a reason for hiding this comment

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

. . . and we won't know until the oal access control tests are run, and even then we won't know if this causes a regression unless we have some way to test Kibana in a wide variety of cases/situations . . .

I guess let's accept this change, and make sure we test as much as possible.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

backtracking here as https://bugzilla.redhat.com/show_bug.cgi?id=1746695 was introduced.. not understood why our access script doesnt expose it

Copy link
Contributor

@richm richm 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 Aug 26, 2019
@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jcantrill, richm

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-merge-robot openshift-merge-robot merged commit 077541b into openshift:master Aug 26, 2019
@openshift-ci-robot
Copy link

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

In response to this:

Bug 1732585: Modify kibana-proxy to rely on ServiceAccount as oauthcl…

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.

@jcantrill
Copy link
Contributor Author

Any cherrypick also needs #241

pmoogi-redhat pushed a commit to pmoogi-redhat/cluster-logging-operator that referenced this pull request Apr 26, 2022
Bug 1732585: Modify kibana-proxy to rely on ServiceAccount as oauthcl…
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. kind/bug Categorizes issue or PR as related to a bug. lgtm Indicates that a PR is ready to be merged. release/4.2 size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants