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

Task: rename env variables for easier reading #405

Open
1 task done
MVrachev opened this issue Sep 29, 2023 · 2 comments
Open
1 task done

Task: rename env variables for easier reading #405

MVrachev opened this issue Sep 29, 2023 · 2 comments
Labels
needs-triage Further discussion is required

Comments

@MVrachev
Copy link
Member

MVrachev commented Sep 29, 2023

What is the task about?

Currently, our environmental variables used for configuration follow the format:
RSTUF_<SERVICE_NAME>_<SERVICE_TYPE>_*
where SERVICE_NAME is something like REDIS, POSTGRESS, AWSS3, etc.
and SERVICE_TYPE can be something like STORAGE, SERVER or KEYVAULT.

Examples:

  • RSTUF_REDIS_SERVER
  • RSTUF_STORAGE_BACKEND
  • RSTUF_LOCAL_STORAGE_BACKEND_PATH
  • RSTUF_AWSS3_STORAGE_ACCESS_KEY
  • RSTUF_AWSKMS_KEYVAULT_KEYS

I suggest that instead of using the format from above we use the format:
RSTUF_<SERVICE_TYPE>_<SERVICE_NAME>*, so all of the above examples will become

  • RSTUF_SERVER_REDIS
  • RSTUF_STORAGE_BACKEND - not changed as this defines what type of service to use => SERVICE_NAME is empty.
  • RSTUF_STORAGE_BACKEND_LOCAL_PATH
  • RSTUF_STORAGE_AWSS3_ACCESS_KEY
  • RSTUF_KEYVAULT_AWSKMS_KEYS

The main argument why I suggest this is that there is an increasing number of env variables we are using and it's a lot easier
to find a specific env variable if the prefix directly tells you for what service type it is: storage, keyvault, server, etc.

The negative is that this will be a breaking change.

What do you think @lukpueh @kairoaraujo?

References

https://repository-service-tuf.readthedocs.io/en/latest/guide/repository-service-tuf-worker/Docker_README.html#environment-variables

Code of Conduct

  • I agree to follow this project's Code of Conduct
@MVrachev MVrachev added the needs-triage Further discussion is required label Sep 29, 2023
@lukpueh
Copy link
Collaborator

lukpueh commented Oct 2, 2023

I don't have strong feelings about this. I can see how your proposal is visually more appealing, but does the new grouping really make more sense? E.g. RSTUF_SERVER_* items don't seem to relate, why group them?

And for STORAGE and KEYVAULT "service types", you are likely to always only use one "service name" in your config anyway, right?

@MVrachev
Copy link
Member Author

MVrachev commented Oct 2, 2023

I don't have strong feelings about this. I can see how your proposal is visually more appealing, but does the new grouping really make more sense? E.g. RSTUF_SERVER_* items don't seem to relate, why group them?

Even for RSTUF_SERVER_* I found it easier to read when I read the prefix and automatically understand what it's about.

And for STORAGE and KEYVAULT "service types", you are likely to always only use one "service name" in your config anyway, right?

Yes, one name for storage and one for keyvault.
Still, because each of them could have multiple env variables (as in the case of AWS) it's visually more pleasing when you can distinguish which are for storage and which for keyvault (especially in the case of AWS where you env variables with a common suffix such as those for access_key, secret_access_key, endpoint_url and region.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-triage Further discussion is required
Projects
None yet
Development

No branches or pull requests

2 participants