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

feat: Add apps proxy local development docs #2187

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Matovidlo
Copy link
Contributor

@Matovidlo Matovidlo commented Dec 17, 2024

Jira: PSGO-919

Changes:

  • Add apps proxy documentation for local development.
  • Include short overview of apps proxy, what is it, why we have it and how to use it.
  • Add basic configuration of the apps proxy for development on local machine.

@Matovidlo Matovidlo force-pushed the feat-add-apps-proxy-local-development-docs branch from 9665c8d to 4b6e991 Compare December 17, 2024 12:59
@Matovidlo Matovidlo marked this pull request as ready for review December 17, 2024 14:07
# Apps proxy Architecture Overview

- Serves for data apps authentication and authorization.
- Typicall usage is to perform OIDC login through some OIDC provider (e.g Microsoft login, google login etc.)
Copy link
Contributor

@jachym-tousek-keboola jachym-tousek-keboola Jan 2, 2025

Choose a reason for hiding this comment

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

Suggested change
- Typicall usage is to perform OIDC login through some OIDC provider (e.g Microsoft login, google login etc.)
- Typicall usage is to perform OIDC login through some OIDC provider (e.g. Microsoft login, Google login etc.)

In project directory run:

```
docker compose run --rm --service-ports dev --net=my-test bash
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't work for me:

Error response from daemon: failed to create task for container: failed to create shim task: OCI runtime create failed: runc create failed: unable to start container process: exec: "--net=my-test": executable file not found in $PATH: unknown

I'm guessing some additional setup is necessary?

Inside this bash run:

```
make run-app-proxy
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't work either.

make: *** No rule to make target 'run-app-proxy'.  Stop.

Looks like a typo.

Suggested change
make run-app-proxy
make run-apps-proxy

Copy link
Contributor

@jachym-tousek-keboola jachym-tousek-keboola Jan 2, 2025

Choose a reason for hiding this comment

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

However make run-apps-proxy doesn't work either:

make[1]: Entering directory '/code'
CGO_ENABLED=0 go build -v -mod mod -ldflags "-s -w" -o "./target/apps-proxy/proxy" ./cmd/apps-proxy
# cd /code; git status --porcelain
fatal: detected dubious ownership in repository at '/code'
To add an exception for this directory, call:

        git config --global --add safe.directory /code
error obtaining VCS status: exit status 128
        Use -buildvcs=false to disable VCS stamping.
make[1]: *** [Makefile:52: build-apps-proxy] Error 1
make[1]: Leaving directory '/code'

The suggested command git config --global --add safe.directory /code fixed it though.

```
docker run --net=cli_default --rm \
--env DOMAIN=test.hub.keboola.local \
--env TARGET_HOST=<containerid> \
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove this extra step to obtain container id. It should be possible to just replace <containerid> with this:

$(docker ps -aqf "name=dev")

Copy link
Contributor

@jachym-tousek-keboola jachym-tousek-keboola left a comment

Choose a reason for hiding this comment

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

Good work, I just want the containerid thing automated.

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.

2 participants