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

Wrong named envvars #7292

Closed
mmattel opened this issue Sep 15, 2023 · 5 comments · Fixed by #7406
Closed

Wrong named envvars #7292

mmattel opened this issue Sep 15, 2023 · 5 comments · Fixed by #7406
Assignees

Comments

@mmattel
Copy link
Contributor

mmattel commented Sep 15, 2023

Just identified when looking into global envvars that should start with OCIS_

STORAGE_USERS_PERMISSION_ENDPOINT --> OCIS_STORAGE_USERS_PERMISSION_ENDPOINT

STORAGE_USERS_ASYNC_PROPAGATOR_PROPAGATION_DELAY --> OCIS_STORAGE_USERS_ASYNC_PROPAGATOR_PROPAGATION_DELAY

In the graph service, same as in userlog, there is the key-pair:
OCIS_MACHINE_AUTH_API_KEY
USERLOG_MACHINE_AUTH_API_KEY --> GRAPH_MACHINE_AUTH_API_KEY

WEB_UI_CONFIG_FILE is double present, can be seen in the web service too, we need to check why.

Following envvars are used in both the userlog and clientlog service. Those should be then either a global one starting with OCIS_ or the clientlog version adapted to CLIENTLOG_:

USERLOG_DEBUG_ADDR
USERLOG_DEBUG_PPROF
USERLOG_DEBUG_TOKEN
USERLOG_DEBUG_ZPAGES
USERLOG_LOG_COLOR
USERLOG_LOG_FILE
USERLOG_LOG_LEVEL
USERLOG_LOG_PRETTY

@kobergj @dragonchaser

@kobergj
Copy link
Collaborator

kobergj commented Sep 18, 2023

STORAGE_USERS_PERMISSION_ENDPOINT and STORAGE_USERS_ASYNC_PROPAGATOR_PROPAGATION_DELAY are only present in storage-users service and therefore need no global OCIS prefix

All MACHINE_AUTH_API_KEY envvars can be deleted as they are no longer used because of service accounts

WEB_UI_CONFIG_FILE is used for different envars. One them needs to removed or renamed!

USERLOG envvars in clientlog service need of course be changed to CLIENTLOG.... That was a copy/paste mistake on implementation and a lazy no-look during review

@rhafer
Copy link
Contributor

rhafer commented Sep 18, 2023

All MACHINE_AUTH_API_KEY envvars can be deleted as they are no longer used because of service accounts

The proxy service is still using machine auth to be able to mint reva tokens for oidc user (and signedurl IIRC). Service accounts won't help to get rid of those cases I think. So we still need MACHINE_AUTH_API_KEY

@kobergj
Copy link
Collaborator

kobergj commented Sep 18, 2023

👍 let me rephrase

For all services except proxy the MACHINE_AUTH_API_KEY envvar can be deleted as it is no longer used because of service accounts.

@kobergj
Copy link
Collaborator

kobergj commented Oct 5, 2023

Missed during ticket creation: auth-machine service needs the MACHINE_AUTH_API_KEY too.

@kobergj
Copy link
Collaborator

kobergj commented Oct 6, 2023

Services idp and frontend also still need the MACHINE_AUTH_API_KEY otherwise tests fail. We need to find out why and replace usage of machine authentication with service authentication. I'll create seperate tickets for that.

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 a pull request may close this issue.

4 participants