-
Notifications
You must be signed in to change notification settings - Fork 57
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
Add path prefixed gitlab-ce instance #905
Add path prefixed gitlab-ce instance #905
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@aryanpingle thanks for the PR. Please check the comments. Given that this compose file is only useful with a properly working OAuth setup, please include those changes in this PR as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@aryanpingle Thanks for the PR. Please see the comments.
* Make https url scheme mandatory for gitlab instance (due to react-oidc) * Hardcode gitlab image for consistency with tests
Please add INTEGRATION.md detailing the steps required for integrating gitlab with the rest of DTaaS installation. It is a three step process
|
@aryanpingle |
Can confirm, authorization fails with a self-signed/invalid certificate (OAuth2 redirect results in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@aryanpingle
In step-1, the DTaaS without any valid OAuth applications. In step-3, these OAuth applications are created on https://foo.com/gitlab. This instance is used to create valid OAuth applications. New step-4 configures DTaaS application again with valid OAuth applications.
In addition, the post-install instructions seem wrong. Please check.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah right, I'll correct the steps. Which post-install instruction are you referring to sir?
> The section "Add TLS Certificates (Optional)" should be treated as | ||
> mandatory for now, to ensure `react-oidc-context` correctly redirects users | ||
> for authorization. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Blank line inside blockquote
will be available on the DTaaS server within the directory: | ||
_deploy/services/gitlab_. | ||
1. Traefik Forward Auth will use the path-prefixed GitLab instance for | ||
authorization on the multi-user installation scenario i.e. `foo.com` (but not on `localhost`). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Line length
- "443:443" | ||
volumes: | ||
- "/var/run/docker.sock:/var/run/docker.sock:ro" | ||
- "${DTAAS_DIR}/deploy/docker/dynamic/tls.local.yml:/etc/traefik/dynamic/tls.yml" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@prasadtalasila Since we've removed forward auth from the localhost setup, I believe we don't need to supply a TLS certificate to run the application over https.
I've verified this by removing this line and deleting the certificates on my machine, and it works fine. Should I remove this line along with the certificates directory and tls.local.yml
?
This reverts commit 141a143.
Code Climate has analyzed commit 7ac6581 and detected 3 issues on this pull request. Here's the issue category breakdown:
View more on Code Climate. |
merged via #961. Thanks @aryanpingle for the contribution. |
PR to address #892 and #899