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

(JWA and notebook controller): Set header needed for R-Studio #5618

Closed
wants to merge 5 commits into from
Closed

(JWA and notebook controller): Set header needed for R-Studio #5618

wants to merge 5 commits into from

Conversation

davidspek
Copy link
Contributor

This PR add a checkbox under the advanced settings section and changes to the notebook-controller that are needed for running R-Studio without Istio. More specifically, when the checkbox is selected, the Istio VirtualService will add the X-RStudio-Root-Path header with the NB_PREFIX to the requests. This is needed to fix the redirect R-Studio does to /auth-sign-in to get a cookie (with this fix, R-Studio redirects to /NB_PREFIX/auth-sign-in as it should). This PR also contains the changes from #5601, but will be reworked once the notebook-controller code is fixed up a bit.

@k8s-ci-robot
Copy link
Contributor

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: DavidSpek
To complete the pull request process, please assign kimwnasptd after the PR has been reviewed.
You can assign the PR to them by writing /assign @kimwnasptd in a comment when ready.

The full list of commands accepted by this bot can be found 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

@google-cla google-cla bot added the cla: yes label Feb 22, 2021
@aws-kf-ci-bot
Copy link

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@davidspek davidspek mentioned this pull request Feb 23, 2021
Closed
4 tasks
@davidspek davidspek changed the title R studio header (JWA and notebook controller): Set header needed for R-Studio Feb 23, 2021
@davidspek davidspek marked this pull request as ready for review February 23, 2021 13:56
This was referenced Feb 23, 2021
Merged
Merged
@google-oss-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: DavidSpek
To complete the pull request process, please assign kimwnasptd after the PR has been reviewed.
You can assign the PR to them by writing /assign @kimwnasptd in a comment when ready.

The full list of commands accepted by this bot can be found 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

@davidspek
Copy link
Contributor Author

davidspek commented Mar 3, 2021 •
edited

/hold
The notebook-controller changes have been moved to #5660. The JWA implementation in #5646 would be preferred over the one provided in this PR.

@davidspek davidspek mentioned this pull request Mar 3, 2021
Merged
@davidspek davidspek closed this Mar 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MartinForReal MartinForReal Awaiting requested review from MartinForReal

@StefanoFioravanzo StefanoFioravanzo Awaiting requested review from StefanoFioravanzo

Assignees
No one assigned
Labels
cla: yes do-not-merge/hold size/M
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@davidspek @k8s-ci-robot @aws-kf-ci-bot @google-oss-robot