Skip to content

add asset signed url proxy logic for docker compose context#1563

Merged
anthony-chaudhary merged 6 commits intomasterfrom
fix-docker-compose-minio
Nov 8, 2023
Merged

add asset signed url proxy logic for docker compose context#1563
anthony-chaudhary merged 6 commits intomasterfrom
fix-docker-compose-minio

Conversation

@sergzak022
Copy link
Copy Markdown
Collaborator

No description provided.

Comment thread local_dispatcher/local_dispatch.py Outdated

class Service():
def __init__(self, name, host, port, path_function, is_default=False):
def __init__(self, name, host, port, path_function, is_default=False, get_service_path=lambda path : path):
Copy link
Copy Markdown
Collaborator Author

@sergzak022 sergzak022 Nov 3, 2023

Choose a reason for hiding this comment

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

get_service_path can be used to modify service path. For example, I use it to remove proxy_to_minio portion from the url before sending it to minio. For all as the other servcies the identity function is used so nothing is modified.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Nov 3, 2023

Unit Test Results

0 tests  ±0   0 ✔️ ±0   0s ⏱️ ±0s
0 suites ±0   0 💤 ±0 
0 files   ±0   0 ±0 

Results for commit f5064ae. ± Comparison against base commit 5219ffa.

♻️ This comment has been updated with latest results.

return path_with_params

def get_request_query_params(self):
return dict(urllib.parse.parse_qsl(urllib.parse.urlsplit(request.url).query))
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

The previous logic that was used to add query params to the request wasn't encoding query params in the url friendly way so it didn't work with minio. get_request_query_params is used to build a dict out of query params so they can be passed as params named argument to requests.requests method.


DOCKER_COMPOSE_CONTEXT = env_adapter.bool(os.getenv('DOCKER_COMPOSE_CONTEXT', False))

if DOCKER_COMPOSE_CONTEXT == True:
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Add minio proxy if DOCKER_COMPOSE_CONTEXT is set.

bucket = bucket)
return signed_url
if settings.DOCKER_COMPOSE_CONTEXT == True:
# note need to proxy request through local dispatcher so users can access from the browser
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

replace minio url with a url path to a proxy if DOCKER_COMPOSE_CONTEXT is true

@anthony-chaudhary
Copy link
Copy Markdown
Member

Thanks Sergey!

self.ingress.app.logger.setLevel(logging.INFO)

self.ingress.app.debug = False
self.ingress.app.debug = True
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@sergzak022 TODO: revert this back to self.ingress.app.debug = False

Comment thread docker-compose.yaml
environment:
- "MINIO_ROOT_USER=${DIFFGRAM_MINIO_ACCESS_KEY_ID}"
- "MINIO_ROOT_PASSWORD=${DIFFGRAM_MINIO_ACCESS_KEY_SECRET}"
healthcheck:
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

createbuckets task still fails for me sometimes with mc: <ERROR> Unable to initialize new alias from the provided credentials. Server not initialized, please try again. error. I'm looking into making a better healthcheck to try to address it.

Comment thread docker-compose.yaml
- walrus
environment:
DOCKER_CONTEXT: True
DOCKER_COMPOSE_CONTEXT: True
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

dispatcher and default need DOCKER_COMPOSE_CONTEXT in order to proxy minio asset urls through dispatcher

Comment thread docker-compose.yaml
depends_on:
- minio
minio:
condition: service_healthy
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Updated createbuckets to wait till minio is healthy.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice

@anthony-chaudhary anthony-chaudhary marked this pull request as ready for review November 8, 2023 21:38
@anthony-chaudhary anthony-chaudhary merged commit 77daaa8 into master Nov 8, 2023
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