244 task move socketmap service from silver repo to raven repo

[Aravinda-HWK]

📌 Description

To move the socketmap service from the silver repo to the raven repo.

• Closes [TASK] Move Socketmap Service from Silver Repo to Raven Repo #244

---

🔍 Changes Made

• Move socketmap service into the internal/socketmap folder
• Create a separate service as cmd/socketmap
• Update the Dockerfile to run the socketmap service separately as well as in the combined image
• Update the entrypoint.sh as well.

✅ Checklist (Email System)

• Core IMAP commands tested (LOGIN, CAPABILITY, LIST, SELECT, FETCH, LOGOUT).
• Authentication is tested.
• Docker build & run validated.
• Configuration loading verified for default and custom paths.
• Persistent storage with Docker volume verified.
• Error handling and logging verified
• Documentation updated (README, config samples).

---

🧪 Testing Instructions

To test the server, use the instructions in the README in the test directory.

---

📷 Screenshots / Logs (if applicable)

---

⚠️ Notes for Reviewers

This should be started to review after merging the #243 PR.

[gemini-code-assist]

Summary of Changes

Hello, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly enhances the Raven mail system by integrating a dedicated socketmap service and introducing group email resolution capabilities. The socketmap service, previously external, is now a core component, providing dynamic Postfix map lookups against the Thunder IDP. Concurrently, the LMTP delivery service has been upgraded to intelligently resolve group email addresses into individual recipients, streamlining mail delivery for organizational groups. These changes improve the system's flexibility and integration with identity management, while also optimizing performance through strategic caching of IDP responses.

Highlights

• Socketmap Service Migration: The socketmap service has been successfully moved from the 'silver' repository to the 'raven' repository, now residing under cmd/socketmap and internal/socketmap.
• New Docker Build Target: A new Docker build target, raven-socketmap, has been introduced, allowing the socketmap service to be deployed and run independently. The main raven image also now includes this service.
• Postfix Integration: The new socketmap service provides dynamic virtual mailbox maps for Postfix, supporting user existence validation, virtual domains, and virtual aliases via the Thunder IDP. This is communicated using the netstring protocol.
• Group Email Resolution: The LMTP delivery service now includes functionality to resolve group emails. If a recipient address matches a defined group email pattern (e.g., engineering-group@domain.com), the service will query the IDP to resolve all members of that group and deliver the email to each individual member.
• IDP Integration and Caching: Both the socketmap service and the LMTP group resolver integrate with the Thunder IDP for authentication, user/domain validation, and group member lookups. Caching mechanisms have been implemented to reduce repeated IDP calls and improve performance.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request introduces a new socketmap service and integrates it into the existing infrastructure, including Docker builds and entrypoint scripts. It also adds a groupresolver package to handle email delivery to groups by querying an IDP. The changes are extensive and well-structured. My review focuses on improving security, robustness, and maintainability. Key feedback points include removing insecure practices like disabling TLS verification and using docker logs for configuration, addressing potential resource leaks, and clarifying placeholder implementations.

[Aravinda-HWK]

/gemini review

[gemini-code-assist]

Code Review

This pull request successfully integrates the socketmap service into the repository. The new service is well-structured, with good practices like retry logic, caching, and security considerations. The integration into the Dockerfile and startup scripts is also well-executed. I've identified a few areas for improvement, including a regression in test coverage, some unhandled errors, and placeholder code that should be addressed. I've also included suggestions for improving performance and consistency.

I am having trouble creating individual review comments. Click here to see my feedback.

internal/server/auth/helpers_internal_test.go (164-191)

The test function TestGetApplicationIDAndReadEnvValue has been removed, but the complex logic it was testing (getApplicationID and readEnvValue) was moved to the internal/conf package without corresponding tests. This is a regression in test coverage. Please move these tests to a new internal/conf/config_test.go file to ensure this critical logic remains tested.

internal/socketmap/handler/alias.go (64)

The function checkAliasInTestDB seems to be a placeholder or test-only implementation, as suggested by its name and limited functionality (only handling postmaster@...). In production code, this function should be renamed to reflect its purpose and ideally be replaced with a proper implementation for alias resolution.

internal/socketmap/thunder/auth.go (99)

The error returned by json.Marshal is being ignored. While marshaling this specific map is unlikely to fail, it's a good practice to always handle errors. An error here would lead to an empty request body being sent, causing unexpected behavior.

	authData, err := json.Marshal(authPayload)
	if err != nil {
		log.Printf("  │ ✗ Failed to marshal auth payload: %v", err)
		log.Printf("  └───────────────────────────────────")
		return nil, fmt.Errorf("failed to marshal auth payload: %w", err)
	}

Dockerfile (157)

The base image for the raven-socketmap stage is alpine:3.19, while the base stage (and other services) use alpine:3.18. It's recommended to use a consistent base image version across all stages to ensure predictable behavior and simplify maintenance. Please consider aligning the Alpine version with the one used in the base stage.

FROM alpine:3.18 AS raven-socketmap

internal/conf/config.go (198)

The regular expression is compiled on every call to extractApplicationIDFromThunderLogs. For better performance, this should be compiled only once at the package level and reused here. You can define it as a package-level variable:

var uuidRegex = regexp.MustCompile(`[a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12}`)

And then use uuidRegex in this function instead of regexp.MustCompile(...).

internal/delivery/groupresolver/groupresolver_test.go (175-178)

This change inlines the logic of the writeResponseJSON helper function. This introduces code duplication, as the helper function is still defined in this file and used elsewhere. It's better to continue using the helper function for consistency and maintainability.

			_ = writeResponseJSON(w, resp)

internal/server/auth/auth.go (534-542)

The logic for retrieving the application ID has been centralized into conf.GetApplicationID(). This is a good refactoring that improves code reuse and maintainability.

[Aravinda-HWK]

Code Review

This pull request successfully integrates the socketmap service into the repository. The new service is well-structured, with good practices like retry logic, caching, and security considerations. The integration into the Dockerfile and startup scripts is also well-executed. I've identified a few areas for improvement, including a regression in test coverage, some unhandled errors, and placeholder code that should be addressed. I've also included suggestions for improving performance and consistency.

I am having trouble creating individual review comments. Click here to see my feedback.

internal/server/auth/helpers_internal_test.go (164-191)

The test function TestGetApplicationIDAndReadEnvValue has been removed, but the complex logic it was testing (getApplicationID and readEnvValue) was moved to the internal/conf package without corresponding tests. This is a regression in test coverage. Please move these tests to a new internal/conf/config_test.go file to ensure this critical logic remains tested.

internal/socketmap/handler/alias.go (64)

The function checkAliasInTestDB seems to be a placeholder or test-only implementation, as suggested by its name and limited functionality (only handling postmaster@...). In production code, this function should be renamed to reflect its purpose and ideally be replaced with a proper implementation for alias resolution.

internal/socketmap/thunder/auth.go (99)

The error returned by json.Marshal is being ignored. While marshaling this specific map is unlikely to fail, it's a good practice to always handle errors. An error here would lead to an empty request body being sent, causing unexpected behavior.

	authData, err := json.Marshal(authPayload)
	if err != nil {
		log.Printf("  │ ✗ Failed to marshal auth payload: %v", err)
		log.Printf("  └───────────────────────────────────")
		return nil, fmt.Errorf("failed to marshal auth payload: %w", err)
	}

Dockerfile (157)

The base image for the raven-socketmap stage is alpine:3.19, while the base stage (and other services) use alpine:3.18. It's recommended to use a consistent base image version across all stages to ensure predictable behavior and simplify maintenance. Please consider aligning the Alpine version with the one used in the base stage.

FROM alpine:3.18 AS raven-socketmap

internal/conf/config.go (198)

The regular expression is compiled on every call to extractApplicationIDFromThunderLogs. For better performance, this should be compiled only once at the package level and reused here. You can define it as a package-level variable:

var uuidRegex = regexp.MustCompile(`[a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12}`)

And then use uuidRegex in this function instead of regexp.MustCompile(...).

internal/delivery/groupresolver/groupresolver_test.go (175-178)

This change inlines the logic of the writeResponseJSON helper function. This introduces code duplication, as the helper function is still defined in this file and used elsewhere. It's better to continue using the helper function for consistency and maintainability.

			_ = writeResponseJSON(w, resp)

internal/server/auth/auth.go (534-542)

The logic for retrieving the application ID has been centralized into conf.GetApplicationID(). This is a good refactoring that improves code reuse and maintainability.

The Gemini comments were resolved.

[Aravinda-HWK]

@maneeshaxyz You can review this PR now.