Skip to content

Conversation

king-alexander
Copy link
Collaborator

@king-alexander king-alexander commented Apr 23, 2025

🗣 Description

Declare a configs top-level element and reference configurations for services in the Compose application.

💭 Motivation and context

This change resolves #80.

🧪 Testing

I tested this change locally with Docker version 4.40.0 (187762) and Docker Compose version v2.34.0-desktop.1. I verified that all services in the project started successfully, then ran existing unit tests. All passed.

✅ Pre-approval checklist

  • This PR has an informative and human-readable title.
  • Changes are limited to a single goal - eschew scope creep!
  • All future TODOs are captured in issues, which are referenced
    in code comments.
  • All relevant type-of-change labels have been added.
  • I have read the CONTRIBUTING document.
  • These code changes follow cisagov code standards.
  • All new and existing tests pass.

✅ Pre-merge checklist

  • Finalize version.

✅ Post-merge checklist

  • Create a release.

The path changed from `/run/secrets/admiral.yaml` to
`/admiral_config`.
Create the `configs` top-level element, grant access to
respective services, and update references.
@king-alexander king-alexander self-assigned this Apr 23, 2025
@king-alexander king-alexander added improvement This issue or pull request will add or improve functionality, maintainability, or ease of use docker Pull requests that update Docker code labels Apr 23, 2025
Reference #82 as future work.
@king-alexander king-alexander marked this pull request as ready for review April 23, 2025 20:23
@dav3r dav3r moved this to In Progress in CyHy System Apr 24, 2025
@dav3r
Copy link
Member

dav3r commented Apr 24, 2025

This PR seems to be removing secrets altogether. I thought the goal here was to move non-sensitive config data into config files and keep sensitive data (e.g. database passwords) in secrets. Or am I misunderstanding something?

@king-alexander
Copy link
Collaborator Author

This PR seems to be removing secrets altogether. I thought the goal here was to move non-sensitive config data into config files and keep sensitive data (e.g. database passwords) in secrets. Or am I misunderstanding something?

The SSLMate API key is still defined as a secret and in #82 I will define a Redis ACL as another. I was going to look into alternative authentication methods for MongoDB as well in order to define the credentials as a separate secret. Should I keep mongo.yaml in the secrets section until I get around to that?

@dav3r
Copy link
Member

dav3r commented Apr 24, 2025

This PR seems to be removing secrets altogether. I thought the goal here was to move non-sensitive config data into config files and keep sensitive data (e.g. database passwords) in secrets. Or am I misunderstanding something?

The SSLMate API key is still defined as a secret and in #82 I will define a Redis ACL as another. I was going to look into alternative authentication methods for MongoDB as well in order to define the credentials as a separate secret. Should I keep mongo.yaml in the secrets section until I get around to that?

Yes, I think anything with a password should remain in the secrets directory. Since admiral.yaml still includes the Redis password, you may want to take care of #82 first, then come back to this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docker Pull requests that update Docker code improvement This issue or pull request will add or improve functionality, maintainability, or ease of use

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

Define a Configs top-level element

2 participants