Skip to content

Major refactoring to make slsm simpler and easier to work with#9

Merged
aarongable merged 1 commit into
mainfrom
refactor
Jul 28, 2025
Merged

Major refactoring to make slsm simpler and easier to work with#9
aarongable merged 1 commit into
mainfrom
refactor

Conversation

@aarongable
Copy link
Copy Markdown
Contributor

This major refactoring -- nearly a rewrite -- of sunlight-secretmanager removes unnecessary layers of complexity, streamlines tests, and makes the project easier to understand and evolve moving forward. It also updates sunlight-secretmanager to target v0.5.0 of Sunlight, which has made several changes to the config format since this project was started.

Specifically:

  • Move all the code directly into //cmd/sunlight-secretmanager, removing the "config" and "secrets" packages, which did not have coherent public interfaces or private implementation details worth protecting.
  • Remove the "nameSeedMap" and "fileNamesMap" concepts, and instead let main directly introspect the loaded config.
  • Remove the "Secrets" and "Filesystem" types, which were unnecessary layers of indirection around the AWS API and an integer, and which made testing more complex.
  • Remove the modular "verifyFilesystem" func, and replace it with simply an integer indicating what type of filesystem should be verified.

This change will be followed by a feature change which adds the ability for sunlight-secretmanager to generate new seeds when it finds a configured log that does not yet have a seed stored in AWS.

@aarongable aarongable force-pushed the refactor branch 4 times, most recently from 62a863b to 94eccc3 Compare July 23, 2025 21:37
@aarongable aarongable marked this pull request as ready for review July 23, 2025 21:59

ctx := context.Background()

cfg, err := awsconfig.LoadDefaultConfig(ctx, awsconfig.WithSharedConfigProfile(os.Getenv("AWS_PROFILE")))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

By default, the SDK checks the AWS_PROFILE environment variable to determine which profile to use

I know you're not touching this line, but I'm pretty sure this WithSharedConfigProfile line isn't needed

@aarongable
Copy link
Copy Markdown
Contributor Author

Merging to unblock the feature work; we can always come back and improve things more in the future.

@aarongable aarongable merged commit ec45efc into main Jul 28, 2025
1 check passed
@aarongable aarongable deleted the refactor branch July 28, 2025 16:11
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