-
Notifications
You must be signed in to change notification settings - Fork 0
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 new "external" app example #90
Conversation
External/README.md
Outdated
# Externally hosted app example | ||
|
||
This example shows how to develop and deploy an externally hosted application that integrates | ||
with Edge's upstream identity provider (Identity/Keycloak) for authentication and retrieval of user metadata. |
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.
it's really much more than integrating with the IdP as it also sets up security, monitoring, access control, etc. I would probably just say "in Edge" in place of "that integrates ...".
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.
I wanted to distinguish this a bit from the "native" Edge app examples in this repo, but I agree that I was underselling it a bit. How does this sound?
3b0c5d3#diff-d2aa805884386c3f7a3abe7aa3c57218e49f57243fda08f3ec3c7cece7d34227R3-R7
External/docker/Dockerfile
Outdated
WORKDIR /home/app | ||
|
||
# Create a default EDM environment using the enthought_edge bundle | ||
RUN edm env import -f /tmp/app_environment.zbundle edm && edm cache purge --yes |
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.
you might want to delete the zbundle to keep the image smaller
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.
Since this is a multi-stage build, the zbundle will never make it into the final image. Feels a bit overkill, but I borrowed this straight from the adjacent native app example.
External/src/main.py
Outdated
# Distribution is prohibited. | ||
|
||
""" | ||
Example Flask application (external). |
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.
remove the (external)
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.
I removed the "external" string from the places you flagged, but it's still showing up in e.g. the example's directory name itself, the DevSpace config, the URL prefix, etc.
Do you prefer to replace all of those with "Kubernetes", or does that feel confusing?
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.
+1 for replacing all the refences to external
External/devspace.yaml
Outdated
- name: "pre-deploy-hook" | ||
command: |- | ||
$!( | ||
if [ ${DEVSPACE_CONTEXT} != "docker-desktop" ] && [ ${DEVSPACE_CONTEXT} != "minikube" ]; then | ||
echo 'aws sts get-caller-identity --profile ${DEVSPACE_AWS_PROFILE} >/dev/null 2>&1 || aws sso login' | ||
else | ||
echo '' | ||
fi | ||
) | ||
events: ["before:deploy"] | ||
- name: "pre-image-build-hook" | ||
command: |- | ||
$!( | ||
if ! [ -f ./src/app_environment.zbundle ] || [ $(get_flag "rebuild-zbundle") == "true" ]; then | ||
echo 'edm bundle generate -i --version 3.8 --platform rh7-x86_64 -m 2.0 -f ./src/app_environment.zbundle ${EDM_APP_DEPENDENCIES}' | ||
else | ||
echo '' | ||
fi | ||
if [ ${DEVSPACE_CONTEXT} != "docker-desktop" ] && [ ${DEVSPACE_CONTEXT} != "minikube" ]; then | ||
echo 'aws sts get-caller-identity --profile ${DEVSPACE_AWS_PROFILE} >/dev/null 2>&1 || aws sso login' | ||
echo 'aws ecr get-login-password --profile ${DEVSPACE_AWS_PROFILE} | docker login --username AWS --password-stdin ${DEVSPACE_AWS_ECR_HOST}' | ||
else | ||
echo '' | ||
fi | ||
) | ||
events: ["before:build"] |
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.
While currently only relevant for remote deployments, the current example config contains a lot of AWS ECR specific logic.
This allows for simple, unified authentication to EKS and ECR via our usual AWS SSO setup, but I'm wondering if this should be vendor-neutral instead (i.e. generic container registry credentials and a K8s pull secret).
External/README.md
Outdated
User metadata is passed to the application via HTTP headers. For the local deployment, we are mocking the headers | ||
by injecting test user metadata into incoming requests via Istio. | ||
|
||
#### Docker Desktop |
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.
do we push Minikube first as a the preferred solution or do we consider Docker Desktop is still important as providing more stability/simplicity/... ?
As discussed, I have
A concern about the second part though: when using the remote deployment option, the current DevSpace configuration still expects a local Docker instance to build and push the images (e.g. to ECR). That works seamlessly with Docker Desktop, but if only Minikube is available locally, we'll probably have to jump through a few extra hoops to get things working. I'll give that a try tomorrow. If that turns out to be a major pain, we might want to switch our "platform" recommendation to Docker Desktop after all ... |
@martinlindner thanks a lot! Regarding the remote deployment needing a local docker instance, I think that's ok. Most remote deployment will be done by pushing to CI and only "expert" users will do it from their local machine. Let's see what your find out when testing it. |
@dpinte Depending on the app/project and its requirements, I think it could make sense to support a variety of deployment "patterns":
Hence my attempt to cover options 1 and 2 in the devspace config for this example. If you think that adds too much complexity, we can of course just rip that part out. Speaking about complexity:
Getting Minikube's built-in Docker daemon to build and push images for a remote deployment was actually not as painful as expected. It basically boils down to:
That worked nicely in combination with the ECR-based remote deployment config in the current |
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.
👍
Adds a new "external" app example.
This is meant to be deployed in tandem with a remote Kubernetes namespace / Oauth2 Proxy configuration set up by the DevOps team, but alternatively supports local "dev" deployments that simply mock user metadata headers.