search

wolfv / quetz-docker

0 stars 2 forks source link

Test https://github.com/wolfv/quetz/tree/testing #9

Open najose opened 10 months ago

najose commented 10 months ago

@wolfv I have tried testing testing the changes in https://github.com/wolfv/quetz/tree/testing and noted my observations below.

Observations

Testing environment

I have a built docker image using the below dockerfile and pushed it a publically accessible registry in case you want to inspect the image I'm using.

Image name: us-east1-docker.pkg.dev/package-factory-sandbox/quetz-sdgr/quetz-testing:latest

# Build conda environment
FROM condaforge/mambaforge:4.9.2-7 as conda

COPY environment.yml /tmp/environment.yml
RUN CONDA_COPY_ALWAYS=true mamba env create -p /env -f /tmp/environment.yml \
  && conda clean -afy

COPY . /code
RUN conda run -p /env python -m pip install --no-deps /code && \
    conda run -p /env python -m pip install /code/plugins/quetz_googleiap && \
    conda run -p /env python -m pip install xattr google-api-python-client google-auth-httplib2 google-auth-oauthlib quetz-frontend gcsfs

# Create image
FROM debian:buster-slim

ENV LANG=C.UTF-8 LC_ALL=C.UTF-8

COPY --from=conda /env /env

# Set WORKDIR to /tmp because quetz always creates a quetz.log file
# in the current directory
WORKDIR /tmp
ENV PATH /env/bin:$PATH
EXPOSE 8000

# The following command assumes that a deployment has been initialized
# in the /quetz-deployment volume
RUN apt-get update -y && \
    apt-get install curl -y

CMD ["quetz", "start", "/quetz-deployment", "--host", "0.0.0.0", "--port", "8000"]

The container startup script creates a deployment and starts Quetz:

quetz create {{ $quetzDeploymentDirectory }} --copy-conf {{ $quetzConfigMapVolMountPath }}/{{ $quetzConfigFileName }} --exists-ok
quetz start {{ $quetzDeploymentDirectory }} --host 0.0.0.0 --port {{ $quetzContainerPort }}

Config

[pamauthenticator]
# name for the provider, used in the login URL
provider = "pam"
# use the following to translate the Unix groups that
# users might belong to user role on Quetz server
admin_groups = ["quetz"]
maintainer_groups = []
user_groups = []

[sqlalchemy]
# In production we use postgres, but the issue can be reproduced with sqlite.
database_url = "sqlite:///./quetz.sqlite"

[session]
# openssl rand -hex 32. The below is not the secret that is actually used :)
secret = "984d92e652ba8ac0801a5a9c8f79b5b0bcf9f3461f65eda7c2867d29a3a276f9"
https_only = false

[logging]
level = "DEBUG"
file = "quetz.log"

[users]
admins = []

[storage]
soft_delete_channel = true
soft_delete_package = true

[googleiam]
server_admin_emails = [ "me@domain.com" ]

[profiling]
enable_sampling = false

[gcs]
project = "package-factory-sandbox"
bucket_prefix = "package-factory-"
bucket_suffix = "-packages"
cache_timeout = "0"
region="us-central1"

I'll add more observations in the GitHub issue as and when I find them. Please let me know when I can test again with the issues resolved in the testing branch.

najose commented 9 months ago

@wolfv, So the above mentioned issue was an error from my side. The persistent volume had the old config, so creating a deployment with --exists-ok must've skipped bringing in the new changes to the configs.

Once I fixed this, I was able to test all the other features as well. I have made some changes but I couldn't push as I don't have write access, so I've added diffs of the changes I made below.

wolfv commented 9 months ago

Thanks, fixed the first issue like you did (and pushed to both testing and the feature branch). Now going to work on the second issue – will also fix up all the tests today.

wolfv commented 9 months ago

@najose I've fixed two additional issues I've encountered for the copying. Maybe you can test with the latest state again :)

wolfv commented 9 months ago

quick note that I implemented the feature you asked for re. skipping the authorization for health endpoints.

najose commented 9 months ago

@wolfv, Thanks will test it out soon.

najose commented 9 months ago

@wolfv, I tested out the latest changes (fix for copying packages, health check with no authN) from the testing branch, the changes are all working as expected.

You seem to have missed one fix for handling server_admin_emails. The current logic seems to be looking at an integer user id in a server_admin_emails list which is supposed to contain a list of emails.


diff --git a/plugins/quetz_googleiap/quetz_googleiap/middleware.py b/plugins/quetz_googleiap/quetz_googleiap/middleware.py
index 71e05c5..cd18b80 100644
--- a/plugins/quetz_googleiap/quetz_googleiap/middleware.py
+++ b/plugins/quetz_googleiap/quetz_googleiap/middleware.py
@@ -91,7 +91,7 @@ class GoogleIAMMiddleware(BaseHTTPMiddleware):
                 )
                 dao.create_channel(channel, user.id, "owner")

-            self.google_role_for_user(user_id, dao)
+            self.google_role_for_user(user_id, email, dao)
             user_id = uuid.UUID(bytes=user.id)
             # drop the db and dao to remove the connection
             del db, dao
@@ -105,15 +105,15 @@ class GoogleIAMMiddleware(BaseHTTPMiddleware):
         response = await call_next(request)
         return response

-    def google_role_for_user(self, user_id, dao):
-        if not user_id:
+    def google_role_for_user(self, user_id, username, dao):
+        if not user_id or not username:
             return

-        if user_id in self.server_admin_emails:
-            logger.info(f"User {user_id} is server admin")
+        if username in self.server_admin_emails:
+            logger.info(f"User '{username}' with user id '{user_id}' is server admin")
             dao.set_user_role(user_id, "owner")
         else:
-            logger.info(f"User {user_id} is not a server admin")
+            logger.info(f"User '{username}' with user id '{user_id}' is not a server admin")
             dao.set_user_role(user_id, "member")