diff --git a/.gitignore b/.gitignore index 5c051c2..b2f4ed7 100644 --- a/.gitignore +++ b/.gitignore @@ -10,3 +10,4 @@ revive.log .claude/ .vscode/settings.json coverage.html +.serena/ diff --git a/CLAUDE.md b/CLAUDE.md index 7f4c333..b21657a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -102,8 +102,9 @@ Environment-based configuration for: #### Environment Variables For local testing with mocks: - `export NATS_URL=nats://localhost:4222` -- `export AUTH_SOURCE=mock` +- `export AUTH_SOURCE=mock` - `export REPOSITORY_SOURCE=mock` +- `export GROUPSIO_SOURCE=mock` - `export JWT_AUTH_DISABLED_MOCK_LOCAL_PRINCIPAL="test-user"` #### Local Kubernetes Deployment with Mock Authentication @@ -136,4 +137,36 @@ For comprehensive integration testing using local Kubernetes cluster: - `values.local.yaml` - Local testing override (mock authentication) - Use `-f values.local.yaml` for local deployment only -**โš ๏ธ Security Warning**: Never use mock authentication in production environments. \ No newline at end of file +**โš ๏ธ Security Warning**: Never use mock authentication in production environments. + +### GroupsIO Integration & Mocking + +#### GroupsIO Client Architecture +The service integrates with Groups.io API through a clean orchestrator pattern: + +```go +// Orchestrator with nil-safe design +type grpsIOWriterOrchestrator struct { + groupsClient *groupsio.Client // May be nil for mock/disabled mode +} + +// Usage pattern throughout service +if o.groupsClient != nil { + result, err := o.groupsClient.CreateGroup(ctx, domain, options) + // Handle Groups.io operations +} else { + // Mock mode: operations bypassed, domain logic continues +} +``` + +#### Mock Configuration +- **Production**: `GROUPSIO_SOURCE=real` - Uses actual Groups.io API client +- **Testing**: `GROUPSIO_SOURCE=mock` - Returns nil client, enables pure domain testing +- **Domain Logic**: All business logic flows through `MockRepository` in `internal/infrastructure/mock/grpsio.go` +- **Error Simulation**: Comprehensive error testing available through domain mock + +#### Benefits of This Pattern +1. **Clean Separation**: Infrastructure (HTTP calls) vs Domain (business logic) +2. **Nil-Safe**: Orchestrator gracefully handles disabled Groups.io integration +3. **Testable**: Domain logic fully tested without external API dependencies +4. **Configurable**: Easy switching between mock and real modes \ No newline at end of file diff --git a/README.md b/README.md index 01c81bc..ca55f76 100644 --- a/README.md +++ b/README.md @@ -1,97 +1,558 @@ -# LFX V2 MailingList Service +# LFX V2 Mailing List Service -This repository contains the source code for the LFX v2 platform mailing list service. +The LFX v2 Mailing List Service is a comprehensive microservice that manages mailing lists and their members within the Linux Foundation's LFX platform. Built with Go and the Goa framework, it provides robust CRUD operations for GroupsIO services, mailing lists, and members with direct Groups.io API integration and NATS JetStream persistence. -## Overview +## ๐Ÿš€ Quick Start -The LFX v2 Mailing List Service is designed to manage mailing lists within the LFX v2 platform. +### For Deployment (Helm) -## File Structure +If you just need to run the service without developing on the service, use the Helm chart: +```bash +# Install the mailing list service +helm upgrade --install lfx-v2-mailing-list-service ./charts/lfx-v2-mailing-list-service \ + --namespace lfx \ + --create-namespace \ + --set image.tag=latest ``` -โ”œโ”€โ”€ bin/ # Compiled binaries -โ”œโ”€โ”€ charts/ # Kubernetes Helm charts -โ”‚ โ””โ”€โ”€ lfx-v2-mailing-list-service/ -โ”‚ โ””โ”€โ”€ templates/ -โ”œโ”€โ”€ cmd/ # Application entry points -โ”‚ โ””โ”€โ”€ mailing-list-api/ -โ”‚ โ”œโ”€โ”€ design/ # GOA API design files -โ”‚ โ”œโ”€โ”€ gen/ # Generated GOA code -โ”‚ โ””โ”€โ”€ service/ # Service implementations -โ”œโ”€โ”€ gen/ # Generated code (GOA) -โ”‚ โ”œโ”€โ”€ http/ -โ”‚ โ””โ”€โ”€ mailing_list/ -โ”œโ”€โ”€ internal/ # Private application code -โ”‚ โ”œโ”€โ”€ domain/ -โ”‚ โ”‚ โ”œโ”€โ”€ model/ # Domain models -โ”‚ โ”‚ โ””โ”€โ”€ port/ # Interface definitions -โ”‚ โ”œโ”€โ”€ infrastructure/ -โ”‚ โ”‚ โ”œโ”€โ”€ auth/ # JWT authentication -โ”‚ โ”‚ โ”œโ”€โ”€ config/ # Configuration -โ”‚ โ”‚ โ”œโ”€โ”€ mock/ # Mock implementations -โ”‚ โ”‚ โ””โ”€โ”€ nats/ # NATS client and messaging -โ”‚ โ”œโ”€โ”€ middleware/ # HTTP middleware -โ”‚ โ””โ”€โ”€ service/ # Business logic layer -โ”œโ”€โ”€ pkg/ # Public packages -โ”‚ โ”œโ”€โ”€ constants/ # Application constants -โ”‚ โ”œโ”€โ”€ errors/ # Custom error types -โ”‚ โ””โ”€โ”€ log/ # Logging utilities -โ””โ”€โ”€ Dockerfile # Container build configuration -``` -## Key Features +### For Local Development + +1. **Prerequisites** + - Go 1.24+ installed + - Make installed + - Docker (optional, for containerized development) + - NATS server running (for local testing) + +2. **Clone and Setup** + + ```bash + git clone https://github.com/linuxfoundation/lfx-v2-mailing-list-service.git + cd lfx-v2-mailing-list-service + + # Install dependencies and generate API code + make deps + make apigen + ``` + +3. **Configure Environment (Optional)** + + ```bash + # For local development without Groups.io + export GROUPSIO_SOURCE=mock + export AUTH_SOURCE=mock + export JWT_AUTH_DISABLED_MOCK_LOCAL_PRINCIPAL="test-admin" + export LOG_LEVEL=debug + ``` + +4. **Run the Service** + + ```bash + # Run with default settings + make run + ``` + +## ๐Ÿ—๏ธ Architecture + +The service is built using a clean architecture pattern with the following layers: + +- **API Layer**: Goa-generated HTTP handlers and OpenAPI specifications +- **Service Layer**: Business logic and orchestration for mailing list operations +- **Domain Layer**: Core business models, entities, and interfaces +- **Infrastructure Layer**: NATS persistence, JWT authentication, and GroupsIO API integration + +### Key Features + +- **GroupsIO Service Management**: Complete CRUD operations for GroupsIO service configurations (primary, formation, shared types) +- **Mailing List Management**: Full lifecycle management of mailing lists/subgroups with comprehensive validation +- **Member Management**: Member operations including delivery modes, moderation status, and subscription management +- **GroupsIO Integration**: Direct Groups.io API integration with authentication, retry logic, and timeout configuration +- **Project Integration**: Mailing lists associated with projects and services for organizational structure +- **NATS JetStream Storage**: Scalable and resilient data persistence across multiple KV buckets +- **NATS Messaging**: Event-driven communication for indexing and access control +- **JWT Authentication**: Secure API access via Heimdall integration +- **Mock Mode**: Complete testing capability without external GroupsIO API dependencies +- **OpenAPI Documentation**: Auto-generated API specifications +- **Comprehensive Testing**: Full unit test coverage with mocks +- **ETag Support**: Optimistic concurrency control for update operations +- **Health Checks**: Built-in `/livez` and `/readyz` endpoints for Kubernetes probes +- **Structured Logging**: JSON-formatted logs with contextual information using Go's slog package -- Health check endpoints for Kubernetes probes -- JWT authentication integration -- NATS messaging and storage support -- Structured logging with Go's slog package -- Docker containerization -- Kubernetes deployment ready +## ๐Ÿ“ Project Structure -## Development +```bash +lfx-v2-mailing-list-service/ +โ”œโ”€โ”€ cmd/ # Application entry points +โ”‚ โ””โ”€โ”€ mailing-list-api/ # Main API server +โ”‚ โ”œโ”€โ”€ design/ # Goa API design files +โ”‚ โ”‚ โ”œโ”€โ”€ mailing_list.go # Service and endpoint definitions +โ”‚ โ”‚ โ””โ”€โ”€ type.go # Type definitions and data structures +โ”‚ โ”œโ”€โ”€ service/ # GOA service implementations +โ”‚ โ”œโ”€โ”€ main.go # Application entry point +โ”‚ โ””โ”€โ”€ http.go # HTTP server setup +โ”œโ”€โ”€ charts/ # Helm chart for Kubernetes deployment +โ”‚ โ””โ”€โ”€ lfx-v2-mailing-list-service/ +โ”‚ โ”œโ”€โ”€ templates/ # Kubernetes resource templates +โ”‚ โ”œโ”€โ”€ values.yaml # Production configuration +โ”‚ โ””โ”€โ”€ values.local.yaml # Local development configuration +โ”œโ”€โ”€ gen/ # Generated code (DO NOT EDIT) +โ”‚ โ”œโ”€โ”€ http/ # HTTP transport layer +โ”‚ โ”‚ โ”œโ”€โ”€ openapi.yaml # OpenAPI 2.0 specification +โ”‚ โ”‚ โ””โ”€โ”€ openapi3.yaml # OpenAPI 3.0 specification +โ”‚ โ””โ”€โ”€ mailing_list/ # Service interfaces +โ”œโ”€โ”€ internal/ # Private application code +โ”‚ โ”œโ”€โ”€ domain/ # Business domain layer +โ”‚ โ”‚ โ”œโ”€โ”€ model/ # Domain models and conversions +โ”‚ โ”‚ โ””โ”€โ”€ port/ # Repository and service interfaces +โ”‚ โ”œโ”€โ”€ service/ # Service layer implementation +โ”‚ โ”‚ โ””โ”€โ”€ grpsio_service_reader.go # GroupsIO service reader +โ”‚ โ”œโ”€โ”€ infrastructure/ # Infrastructure layer +โ”‚ โ”‚ โ”œโ”€โ”€ auth/ # JWT authentication +โ”‚ โ”‚ โ”œโ”€โ”€ groupsio/ # GroupsIO API client implementation +โ”‚ โ”‚ โ”œโ”€โ”€ nats/ # NATS messaging and storage +โ”‚ โ”‚ โ”‚ โ”œโ”€โ”€ messaging_publish.go # Message publishing +โ”‚ โ”‚ โ”‚ โ”œโ”€โ”€ messaging_request.go # Request/reply messaging +โ”‚ โ”‚ โ”‚ โ””โ”€โ”€ storage.go # KV store repositories +โ”‚ โ”‚ โ””โ”€โ”€ mock/ # Mock implementations for testing +โ”‚ โ”‚ โ”œโ”€โ”€ auth.go # Mock authentication +โ”‚ โ”‚ โ””โ”€โ”€ grpsio.go # Mock GroupsIO repository +โ”‚ โ””โ”€โ”€ middleware/ # HTTP middleware components +โ”‚ โ”œโ”€โ”€ authorization.go # JWT-based authorization +โ”‚ โ””โ”€โ”€ request_id.go # Request ID injection +โ”œโ”€โ”€ pkg/ # Public packages +โ”‚ โ”œโ”€โ”€ constants/ # Application constants +โ”‚ โ”‚ โ”œโ”€โ”€ context.go # Context keys +โ”‚ โ”‚ โ”œโ”€โ”€ global.go # Global constants +โ”‚ โ”‚ โ”œโ”€โ”€ storage.go # Storage bucket names +โ”‚ โ”‚ โ””โ”€โ”€ subjects.go # NATS subject definitions +โ”‚ โ”œโ”€โ”€ errors/ # Error types +โ”‚ โ””โ”€โ”€ utils/ # Utility functions +โ”œโ”€โ”€ Dockerfile # Container build configuration +โ”œโ”€โ”€ Makefile # Build and development commands +โ”œโ”€โ”€ CLAUDE.md # Claude Code assistant instructions +โ””โ”€โ”€ go.mod # Go module definition +``` + +## ๐Ÿ› ๏ธ Development ### Prerequisites + - Go 1.24+ -- GOA v3 -- Docker (optional) +- Make +- Git ### Getting Started -1. **Clone the repository:** +1. **Install Dependencies** + ```bash - git clone https://github.com/linuxfoundation/lfx-v2-mailing-list-service.git - cd lfx-v2-mailing-list-service + make deps ``` -2. **Install dependencies:** + This installs: + - Go module dependencies + - Goa CLI for code generation + +2. **Generate API Code** + ```bash - go mod tidy + make apigen ``` -3. **Generate API code:** + Generates HTTP transport, client, and OpenAPI documentation from design files. + +3. **Build the Application** + ```bash - make generate + make build ``` -4. **Build the application:** + Creates the binary in `bin/lfx-v2-mailing-list-service`. + +### Development Workflow + +#### Running the Service + +```bash +# Run with auto-regeneration +make run + +# Build and run binary +make build +./bin/lfx-v2-mailing-list-service +``` + +#### Code Quality + +**Always run these before committing:** + +```bash +# Run linter +make lint + +# Run all tests +make test + +# Run complete pipeline (setup + lint + test + build) +make all +``` + +#### Testing + +```bash +# Run all tests with race detection and coverage +make test + +# View coverage report +go tool cover -html=coverage.out +``` + +**Writing Tests:** + +- Place test files alongside source files with `_test.go` suffix +- Use table-driven tests for multiple test cases +- Mock external dependencies using the provided mock interfaces in `internal/infrastructure/mock/` +- Achieve high test coverage (aim for >80%) +- Test both happy path and error cases + +Example test structure: + +```go +func TestServiceMethod(t *testing.T) { + tests := []struct { + name string + input InputType + setupMocks func(*MockRepository) + expected ExpectedType + expectError bool + }{ + // Test cases here + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Test implementation + }) + } +} +``` + +#### API Development + +When modifying the API: + +1. **Update Design Files** in `cmd/mailing-list-api/design/` directory +2. **Regenerate Code**: + ```bash - make build + make apigen ``` -5. **Run the service:** +3. **Run Tests** to ensure nothing breaks: + ```bash - make run + make test ``` -### Available Commands +4. **Update Service Implementation** in `cmd/mailing-list-api/service/` + +#### GroupsIO Integration Development + +The GroupsIO integration follows a clean orchestrator pattern: + +**Architecture Pattern:** + +```go +// Orchestrator with nil-safe design +type grpsIOWriterOrchestrator struct { + groupsClient *groupsio.Client // May be nil for mock/disabled mode +} + +// Usage pattern throughout service +if o.groupsClient != nil { + result, err := o.groupsClient.CreateGroup(ctx, domain, options) + // Handle Groups.io operations +} else { + // Mock mode: operations bypassed, domain logic continues +} +``` + +**Configuration Modes:** + +- **Production**: `GROUPSIO_SOURCE=real` - Uses actual Groups.io API client +- **Testing**: `GROUPSIO_SOURCE=mock` - Returns nil client, enables pure domain testing +- **Domain Logic**: All business logic flows through `MockRepository` in `internal/infrastructure/mock/grpsio.go` + +**Benefits:** +1. **Clean Separation**: Infrastructure (HTTP calls) vs Domain (business logic) +2. **Nil-Safe**: Orchestrator gracefully handles disabled Groups.io integration +3. **Testable**: Domain logic fully tested without external API dependencies +4. **Configurable**: Easy switching between mock and real modes + +### Available Make Targets + +| Target | Description | +|--------|-------------| +| `make all` | Complete build pipeline (setup, lint, test, build) | +| `make deps` | Install dependencies and Goa CLI | +| `make setup` | Setup development environment | +| `make setup-dev` | Install development tools (golangci-lint) | +| `make apigen` | Generate API code from design files | +| `make build` | Build the binary | +| `make run` | Run the service locally | +| `make test` | Run unit tests with race detection | +| `make lint` | Run code linter | +| `make clean` | Remove build artifacts | +| `make docker-build` | Build Docker image | +| `make docker-run` | Run Docker container locally | +| `make helm-install` | Install Helm chart | +| `make helm-install-local` | Install with mock authentication | +| `make helm-templates` | Print Helm templates | +| `make helm-uninstall` | Uninstall Helm chart | + +## ๐Ÿงช Testing -- `make build` - Build the application -- `make generate` - Generate GOA code from design -- `make run` - Run the service locally -- `make test` - Run tests -- `make clean` - Clean build artifacts +### Running Tests -## License +```bash +# Run all tests +make test + +# Run specific package tests +go test -v ./internal/service/... + +# Run with coverage +go test -v -race -coverprofile=coverage.out ./... +go tool cover -html=coverage.out +``` + +### Test Structure + +The project follows Go testing best practices: + +- **Unit Tests**: Test individual components in isolation +- **Integration Tests**: Test component interactions +- **Mock Interfaces**: Located in `internal/infrastructure/mock/` +- **Test Coverage**: Aim for high coverage with meaningful tests + +### Writing Tests + +When adding new functionality: + +1. **Write tests first** (TDD approach recommended) +2. **Use table-driven tests** for multiple scenarios +3. **Mock external dependencies** using provided interfaces +4. **Test error conditions** not just happy paths +5. **Keep tests focused** and independent + +### Local Testing with Mock Authentication + +For comprehensive integration testing using local Kubernetes cluster: + +1. **Deploy with Mock Authentication**: + ```bash + make helm-install-local + ``` + This deploys the service with: + - `AUTH_SOURCE=mock` - Bypasses JWT validation + - `JWT_AUTH_DISABLED_MOCK_LOCAL_PRINCIPAL=test-super-admin` - Mock principal + - Mock GroupsIO integration + +2. **Test Individual Endpoints**: + ```bash + # Any Bearer token works with mock auth + curl -H "Authorization: Bearer test-token" \ + http://lfx-v2-mailing-list-service.lfx.svc.cluster.local:8080/groupsio/services?v=1 + ``` + +**โš ๏ธ Security Warning**: Never use mock authentication in production environments. + +## ๐Ÿš€ Deployment + +### Helm Chart + +The service includes a Helm chart for Kubernetes deployment: + +```bash +# Install with default values +make helm-install + +# Install with custom values +helm upgrade --install lfx-v2-mailing-list-service ./charts/lfx-v2-mailing-list-service \ + --namespace lfx \ + --values custom-values.yaml + +# Install with GroupsIO credentials +helm upgrade --install lfx-v2-mailing-list-service ./charts/lfx-v2-mailing-list-service \ + --namespace lfx \ + --set groupsio.email="your-email@example.com" \ + --set groupsio.password="your-password" + +# View templates +make helm-templates +``` + +### Docker + +```bash +# Build Docker image +make docker-build + +# Run with Docker +docker run -p 8080:8080 linuxfoundation/lfx-v2-mailing-list-service:latest +``` + +## ๐Ÿ“ก NATS Messaging + +The service uses NATS for event-driven communication with other LFX platform services. + +### Published Subjects + +The service publishes messages to the following NATS subjects: + +| Subject | Purpose | Message Schema | +|---------|---------|----------------| +| `lfx.index.groupsio_service` | GroupsIO service indexing events | Indexer message with tags | +| `lfx.index.groupsio_mailing_list` | Mailing list indexing events | Indexer message with tags | +| `lfx.index.groupsio_member` | Member indexing events | Indexer message with tags | +| `lfx.update_access.groupsio_service` | Service access control updates | Access control message | +| `lfx.delete_all_access.groupsio_service` | Service access control deletion | Access control message | +| `lfx.update_access.groupsio_mailing_list` | Mailing list access control updates | Access control message | +| `lfx.delete_all_access.groupsio_mailing_list` | Mailing list access control deletion | Access control message | + +### Request/Reply Subjects + +The service handles incoming requests on these subjects: + +| Subject | Purpose | +|---------|---------| +| `lfx.projects-api.get_slug` | Project slug requests | +| `lfx.projects-api.get_name` | Project name requests | +| `lfx.committee-api.get_name` | Committee name requests | + +### Message Publisher Interface + +The service uses two message types: + +- **Indexer Messages**: For search indexing operations (consumed by indexer services) +- **Access Messages**: For permission management (consumed by fga-sync service) + +### Event Flow + +When services, mailing lists, or members are modified, the service automatically: + +1. **Updates NATS KV storage** for persistence +2. **Publishes indexing messages** for search services +3. **Publishes access control messages** for permission services +4. **Handles cleanup messages** for cascading deletions + +## ๐Ÿ“– API Documentation + +The service automatically generates OpenAPI documentation: + +- **OpenAPI 2.0**: `gen/http/openapi.yaml` +- **OpenAPI 3.0**: `gen/http/openapi3.yaml` +- **JSON formats**: Also available in `gen/http/` + +Access the documentation at: `http://localhost:8080/openapi.json` + +### Available Endpoints + +| Endpoint | Method | Description | +|----------|--------|-------------| +| `/livez` | GET | Health check | +| `/readyz` | GET | Readiness check | +| `/groupsio/services` | GET, POST | List/create GroupsIO services | +| `/groupsio/services/{uid}` | GET, PUT, DELETE | Get/update/delete service | +| `/groupsio/mailing-lists` | POST | Create mailing list | +| `/groupsio/mailing-lists/{uid}` | GET, PUT, DELETE | Get/update/delete mailing list | +| `/groupsio/mailing-lists/{uid}/members` | GET, POST | List/create members | +| `/groupsio/mailing-lists/{uid}/members/{member_uid}` | GET, PUT, DELETE | Get/update/delete member | + +## ๐Ÿ”ง Configuration + +The service can be configured via environment variables: + +### Core Service Configuration + +| Variable | Description | Default | +|----------|-------------|---------| +| `NATS_URL` | NATS server URL | `nats://lfx-platform-nats.lfx.svc.cluster.local:4222` | +| `LOG_LEVEL` | Log level (debug, info, warn, error) | `info` | +| `LOG_ADD_SOURCE` | Add source location to logs | `true` | +| `PORT` | HTTP server port | `8080` | + +### Authentication Configuration + +| Variable | Description | Default | +|----------|-------------|---------| +| `JWKS_URL` | JWKS URL for JWT verification | `http://lfx-platform-heimdall.lfx.svc.cluster.local:4457/.well-known/jwks` | +| `JWT_AUDIENCE` | JWT token audience | `lfx-v2-mailing-list-service` | +| `AUTH_SOURCE` | Authentication source (`jwt` or `mock`) | `jwt` | +| `JWT_AUTH_DISABLED_MOCK_LOCAL_PRINCIPAL` | Mock principal for local dev (dev only) | `""` | +| `SKIP_ETAG_VALIDATION` | Skip ETag validation (dev only) | `false` | + +### GroupsIO Integration Configuration + +| Variable | Description | Default | +|----------|-------------|---------| +| `GROUPSIO_EMAIL` | Groups.io account email for authentication | Required for production | +| `GROUPSIO_PASSWORD` | Groups.io account password for authentication | Required for production | +| `GROUPSIO_BASE_URL` | Groups.io API base URL | `https://groups.io/api` | +| `GROUPSIO_TIMEOUT` | HTTP timeout for Groups.io API calls | `30s` | +| `GROUPSIO_MAX_RETRIES` | Maximum retry attempts for failed requests | `3` | +| `GROUPSIO_RETRY_DELAY` | Delay between retry attempts | `1s` | +| `GROUPSIO_SOURCE` | Set to `mock` to disable real Groups.io calls | `""` | + +### GroupsIO Domain Configuration + +The Groups.io domain can be specified in two ways: + +1. **API Field Parameter (Recommended)**: Pass the `domain` field in service creation requests +2. **Default**: Uses `groups.io` if no domain is specified + +#### Sandbox Testing with Linux Foundation Groups.io + +**Important**: For sandbox testing with Linux Foundation's Groups.io tenant, you **must** specify the domain as `linuxfoundation.groups.io` in your API requests. + +Example service creation with domain: + +```bash +curl -X POST "localhost:8080/groupsio/services?v=1" \ + -H "Content-Type: application/json" \ + -d '{ + "project_uid": "550e8400-e29b-41d4-a716-446655440000", + "type": "primary", + "domain": "linuxfoundation.groups.io", + "global_owners": ["admin@example.com"], + "project_name": "Test Project" + }' +``` + +### Development Environment Variables + +For local development with Groups.io integration: + +```bash +export GROUPSIO_EMAIL="your-groups-io-email@example.com" +export GROUPSIO_PASSWORD="your-groups-io-password" +export AUTH_SOURCE="mock" +export JWT_AUTH_DISABLED_MOCK_LOCAL_PRINCIPAL="test-admin" +export LOG_LEVEL="debug" +``` + +For local development without Groups.io: + +```bash +export GROUPSIO_SOURCE="mock" +export AUTH_SOURCE="mock" +export JWT_AUTH_DISABLED_MOCK_LOCAL_PRINCIPAL="test-admin" +export LOG_LEVEL="debug" +export NATS_URL="nats://localhost:4222" +``` + +## ๐Ÿ“„ License Copyright The Linux Foundation and each contributor to LFX. -SPDX-License-Identifier: MIT + +SPDX-License-Identifier: MIT \ No newline at end of file diff --git a/charts/lfx-v2-mailing-list-service/templates/deployment.yaml b/charts/lfx-v2-mailing-list-service/templates/deployment.yaml index a6ddeb4..6d6811e 100644 --- a/charts/lfx-v2-mailing-list-service/templates/deployment.yaml +++ b/charts/lfx-v2-mailing-list-service/templates/deployment.yaml @@ -37,6 +37,21 @@ spec: value: {{.Values.app.authSource}} - name: JWT_AUTH_DISABLED_MOCK_LOCAL_PRINCIPAL value: {{.Values.app.jwtAuthDisabledMockLocalPrincipal}} + # GroupsIO configuration + - name: GROUPSIO_SOURCE + value: {{.Values.app.groupsio.source}} + - name: GROUPSIO_BASE_URL + value: {{.Values.app.groupsio.baseUrl}} + - name: GROUPSIO_EMAIL + value: {{.Values.app.groupsio.email}} + - name: GROUPSIO_PASSWORD + value: {{.Values.app.groupsio.password}} + - name: GROUPSIO_TIMEOUT + value: {{.Values.app.groupsio.timeout}} + - name: GROUPSIO_MAX_RETRIES + value: {{.Values.app.groupsio.maxRetries | quote}} + - name: GROUPSIO_RETRY_DELAY + value: {{.Values.app.groupsio.retryDelay}} ports: - containerPort: {{.Values.service.port}} name: web @@ -57,4 +72,4 @@ spec: path: /readyz port: web failureThreshold: 30 - periodSeconds: 1 \ No newline at end of file + periodSeconds: 1 diff --git a/charts/lfx-v2-mailing-list-service/templates/httproute.yaml b/charts/lfx-v2-mailing-list-service/templates/httproute.yaml index 255fb53..6e3d19e 100644 --- a/charts/lfx-v2-mailing-list-service/templates/httproute.yaml +++ b/charts/lfx-v2-mailing-list-service/templates/httproute.yaml @@ -4,31 +4,31 @@ apiVersion: gateway.networking.k8s.io/v1 kind: HTTPRoute metadata: - name: {{.Chart.Name}} - namespace: {{.Values.lfx.namespace}} + name: {{ .Chart.Name }} + namespace: {{ .Values.lfx.namespace }} spec: parentRefs: - - name: {{ .Values.traefik.gateway.name }} - namespace: {{ .Values.traefik.gateway.namespace }} + - name: {{ .Values.traefik.gateway.name }} + namespace: {{ .Values.traefik.gateway.namespace }} hostnames: - - "lfx-api.{{ .Values.lfx.domain }}" + - "lfx-api.{{ .Values.lfx.domain }}" rules: - # Main application endpoints (with authentication) - - matches: - - path: - type: Exact - value: /groupsio - - path: - type: PathPrefix - value: /groupsio/ - {{- if .Values.heimdall.enabled }} - filters: - - type: ExtensionRef - extensionRef: - group: traefik.io - kind: Middleware - name: heimdall-forward-body - {{- end }} - backendRefs: - - name: {{ .Chart.Name }} - port: {{ .Values.service.port }} + # Main application endpoints (with authentication) + - matches: + - path: + type: Exact + value: /groupsio + - path: + type: PathPrefix + value: /groupsio/ + {{- if .Values.heimdall.enabled }} + filters: + - type: ExtensionRef + extensionRef: + group: traefik.io + kind: Middleware + name: heimdall-forward-body + {{- end }} + backendRefs: + - name: {{ .Chart.Name }} + port: {{ .Values.service.port }} diff --git a/charts/lfx-v2-mailing-list-service/templates/nats-kv-buckets.yaml b/charts/lfx-v2-mailing-list-service/templates/nats-kv-buckets.yaml index c370938..3c24d41 100644 --- a/charts/lfx-v2-mailing-list-service/templates/nats-kv-buckets.yaml +++ b/charts/lfx-v2-mailing-list-service/templates/nats-kv-buckets.yaml @@ -1,7 +1,7 @@ # Copyright The Linux Foundation and each contributor to LFX. # SPDX-License-Identifier: MIT --- -{{- if .Values.nats.groupsio_services_kv_bucket.creation}} +{{- if .Values.nats.groupsio_services_kv_bucket.creation }} apiVersion: jetstream.nats.io/v1beta2 kind: KeyValue metadata: @@ -56,4 +56,4 @@ spec: maxValueSize: {{ .Values.nats.groupsio_members_kv_bucket.maxValueSize }} maxBytes: {{ .Values.nats.groupsio_members_kv_bucket.maxBytes }} compression: {{ .Values.nats.groupsio_members_kv_bucket.compression }} -{{- end }} \ No newline at end of file +{{- end }} diff --git a/charts/lfx-v2-mailing-list-service/templates/ruleset.yaml b/charts/lfx-v2-mailing-list-service/templates/ruleset.yaml index 7a71dc9..814ac5a 100644 --- a/charts/lfx-v2-mailing-list-service/templates/ruleset.yaml +++ b/charts/lfx-v2-mailing-list-service/templates/ruleset.yaml @@ -18,7 +18,7 @@ spec: execute: - authenticator: oidc - authenticator: anonymous_authenticator - {{- if .Values.app.use_oidc_contextualizer}} + {{- if .Values.app.use_oidc_contextualizer }} - contextualizer: oidc_contextualizer {{- end }} {{- if .Values.openfga.enabled }} @@ -138,7 +138,7 @@ spec: {{- end }} {{- if .Values.openfga.enabled }} - authorizer: json_content_type - + - authorizer: openfga_check config: values: @@ -245,7 +245,7 @@ spec: {{- end }} {{- if .Values.openfga.enabled }} - authorizer: json_content_type - + - authorizer: openfga_check config: values: diff --git a/charts/lfx-v2-mailing-list-service/values.local.yaml b/charts/lfx-v2-mailing-list-service/values.local.yaml index bc74c98..9949dd9 100644 --- a/charts/lfx-v2-mailing-list-service/values.local.yaml +++ b/charts/lfx-v2-mailing-list-service/values.local.yaml @@ -13,6 +13,9 @@ app: jwtAuthDisabledMockLocalPrincipal: "test-super-admin" # Enable debug logging for local development logLevel: debug + # Use mock GroupsIO integration for local testing + groupsioSource: mock + # Disable OpenFGA authorization for local testing openfga: diff --git a/charts/lfx-v2-mailing-list-service/values.yaml b/charts/lfx-v2-mailing-list-service/values.yaml index a650ed8..a982e56 100644 --- a/charts/lfx-v2-mailing-list-service/values.yaml +++ b/charts/lfx-v2-mailing-list-service/values.yaml @@ -134,3 +134,22 @@ app: # jwt is the configuration for JWT authentication # audience is the intended audience for the JWT token audience: lfx-v2-mailing-list-service + + # GroupsIO integration configuration + groupsio: + # source determines the GroupsIO implementation (groupsio or mock) + source: groupsio + # baseUrl is the Groups.io API base URL + baseUrl: "https://groups.io/api" + # email is the Groups.io account email for authentication + # Do not commit actual credentials to this file will be set by Kubernetes secrets + email: "" + # password is the Groups.io account password for authentication + # Do not commit actual credentials to this file will be set by Kubernetes secrets + password: "" + # timeout is the HTTP client timeout for Groups.io requests + timeout: "30s" + # maxRetries is the maximum number of retry attempts for failed requests + maxRetries: 3 + # retryDelay is the delay between retry attempts + retryDelay: "1s" diff --git a/cmd/mailing-list-api/design/type.go b/cmd/mailing-list-api/design/type.go index 529e0b3..6e93280 100644 --- a/cmd/mailing-list-api/design/type.go +++ b/cmd/mailing-list-api/design/type.go @@ -56,7 +56,6 @@ func GrpsIOServiceBaseAttributes() { dsl.Required("type", "project_uid") } - // GrpsIOServiceWithReadonlyAttributes is the DSL type for a GroupsIO service with readonly attributes. var GrpsIOServiceWithReadonlyAttributes = dsl.Type("grps-io-service-with-readonly-attributes", func() { dsl.Description("A representation of GroupsIO services with readonly attributes.") @@ -295,7 +294,6 @@ func GrpsIOMailingListBaseAttributes() { } - // GrpsIOMailingListUIDAttribute is the DSL attribute for mailing list UID. func GrpsIOMailingListUIDAttribute() { dsl.Attribute("uid", dsl.String, "Mailing list UID -- unique identifier for the mailing list", func() { @@ -528,4 +526,3 @@ func GrpsIOMemberUpdateAttributes() { dsl.Default("none") }) } - diff --git a/cmd/mailing-list-api/service/mailing_list_service.go b/cmd/mailing-list-api/service/mailing_list_service.go index 303c7ea..414368c 100644 --- a/cmd/mailing-list-api/service/mailing_list_service.go +++ b/cmd/mailing-list-api/service/mailing_list_service.go @@ -593,10 +593,10 @@ func payloadStringValue(val *string) string { return *val } -// payloadInt64Value safely extracts int64 value from payload pointer -func payloadInt64Value(val *int64) int64 { +// payloadInt64Ptr safely converts int64 pointer from payload to nullable pointer for domain model +func payloadInt64Ptr(val *int64) *int64 { if val == nil { - return 0 + return nil } - return *val + return val } diff --git a/cmd/mailing-list-api/service/providers.go b/cmd/mailing-list-api/service/providers.go index 00fae4b..aaa11e6 100644 --- a/cmd/mailing-list-api/service/providers.go +++ b/cmd/mailing-list-api/service/providers.go @@ -14,6 +14,7 @@ import ( "github.com/linuxfoundation/lfx-v2-mailing-list-service/internal/domain/port" "github.com/linuxfoundation/lfx-v2-mailing-list-service/internal/infrastructure/auth" + "github.com/linuxfoundation/lfx-v2-mailing-list-service/internal/infrastructure/groupsio" infrastructure "github.com/linuxfoundation/lfx-v2-mailing-list-service/internal/infrastructure/mock" "github.com/linuxfoundation/lfx-v2-mailing-list-service/internal/infrastructure/nats" "github.com/linuxfoundation/lfx-v2-mailing-list-service/internal/service" @@ -23,8 +24,10 @@ var ( natsStorageClient port.GrpsIOReaderWriter natsMessagingClient port.EntityAttributeReader natsPublisherClient port.MessagePublisher + groupsIOClient groupsio.ClientInterface - natsDoOnce sync.Once + natsDoOnce sync.Once + groupsIOClientOnce sync.Once ) func natsInit(ctx context.Context) { @@ -274,6 +277,33 @@ func MessagePublisher(ctx context.Context) port.MessagePublisher { return publisher } +// GroupsIOClient initializes the GroupsIO client with singleton pattern +func GroupsIOClient(ctx context.Context) groupsio.ClientInterface { + groupsIOClientOnce.Do(func() { + // Check if we should use mock + if os.Getenv("GROUPSIO_SOURCE") == "mock" { + slog.InfoContext(ctx, "using mock GroupsIO client") + groupsIOClient = infrastructure.NewMockGroupsIOClient() + return + } + + // Real client initialization - Groups.io integration + config := groupsio.NewConfigFromEnv() + + client, err := groupsio.NewClient(config) + if err != nil { + slog.ErrorContext(ctx, "failed to initialize GroupsIO client - this service requires Groups.io integration", "error", err) + // Don't fatal - service can run without GroupsIO for local development + return + } + + groupsIOClient = client + slog.InfoContext(ctx, "GroupsIO client initialized successfully") + }) + + return groupsIOClient +} + // GrpsIOReaderOrchestrator initializes the service reader orchestrator func GrpsIOReaderOrchestrator(ctx context.Context) service.GrpsIOReader { grpsIOReader := GrpsIOReader(ctx) @@ -289,11 +319,13 @@ func GrpsIOWriterOrchestrator(ctx context.Context) service.GrpsIOWriter { grpsIOReader := GrpsIOReader(ctx) entityReader := EntityAttributeRetriever(ctx) publisher := MessagePublisher(ctx) + groupsClient := GroupsIOClient(ctx) // May be nil for mock/disabled return service.NewGrpsIOWriterOrchestrator( service.WithGrpsIOWriter(grpsIOWriter), service.WithGrpsIOWriterReader(grpsIOReader), service.WithEntityAttributeReader(entityReader), service.WithPublisher(publisher), + service.WithGroupsIOClient(groupsClient), ) } diff --git a/cmd/mailing-list-api/service/service_payload_converters.go b/cmd/mailing-list-api/service/service_payload_converters.go index 1d300ef..e1cf76f 100644 --- a/cmd/mailing-list-api/service/service_payload_converters.go +++ b/cmd/mailing-list-api/service/service_payload_converters.go @@ -22,7 +22,7 @@ func (s *mailingListService) convertGrpsIOServiceCreatePayloadToDomain(p *mailin service := &model.GrpsIOService{ Type: p.Type, Domain: payloadStringValue(p.Domain), - GroupID: payloadInt64Value(p.GroupID), + GroupID: payloadInt64Ptr(p.GroupID), Status: payloadStringValue(p.Status), GlobalOwners: p.GlobalOwners, Prefix: payloadStringValue(p.Prefix), diff --git a/cmd/mailing-list-api/service/service_payload_converters_test.go b/cmd/mailing-list-api/service/service_payload_converters_test.go index ce057d2..02568a5 100644 --- a/cmd/mailing-list-api/service/service_payload_converters_test.go +++ b/cmd/mailing-list-api/service/service_payload_converters_test.go @@ -38,7 +38,7 @@ func TestConvertCreatePayloadToDomain(t *testing.T) { expected: &model.GrpsIOService{ Type: "primary", Domain: "example.groups.io", - GroupID: 12345, + GroupID: int64Ptr(12345), Status: "active", GlobalOwners: []string{"owner1@example.com", "owner2@example.com"}, Prefix: "test-prefix", @@ -61,7 +61,7 @@ func TestConvertCreatePayloadToDomain(t *testing.T) { expected: &model.GrpsIOService{ Type: "formation", Domain: "", - GroupID: 0, + GroupID: nil, Status: "", GlobalOwners: nil, Prefix: "", @@ -218,7 +218,7 @@ func TestConvertUpdatePayloadToDomain(t *testing.T) { Type: "primary", UID: "service-123", Domain: "example.groups.io", - GroupID: 12345, + GroupID: int64Ptr(12345), Prefix: "", ProjectSlug: "test-project", ProjectName: "Test Project", @@ -241,7 +241,7 @@ func TestConvertUpdatePayloadToDomain(t *testing.T) { Type: "primary", UID: "service-123", Domain: "example.groups.io", - GroupID: 12345, + GroupID: int64Ptr(12345), Status: "inactive", GlobalOwners: []string{"newowner@example.com"}, Prefix: "", @@ -264,7 +264,7 @@ func TestConvertUpdatePayloadToDomain(t *testing.T) { Type: "formation", UID: "service-456", Domain: "test.groups.io", - GroupID: 67890, + GroupID: int64Ptr(67890), ProjectUID: "project-456", ProjectSlug: "test-formation", CreatedAt: baseTime, @@ -277,7 +277,7 @@ func TestConvertUpdatePayloadToDomain(t *testing.T) { Type: "formation", UID: "service-456", Domain: "test.groups.io", - GroupID: 67890, + GroupID: int64Ptr(67890), Status: "", ProjectUID: "project-456", ProjectSlug: "test-formation", @@ -345,8 +345,8 @@ func TestConvertMemberUpdatePayloadToDomain(t *testing.T) { LastName: "User", Organization: "Existing Corp", JobTitle: "Existing Engineer", - DeliveryMode: "digest", // Existing preference - ModStatus: "moderator", // Existing permission + DeliveryMode: "digest", // Existing preference + ModStatus: "moderator", // Existing permission MemberType: "direct", Status: "active", CreatedAt: now.Add(-24 * time.Hour), @@ -371,15 +371,15 @@ func TestConvertMemberUpdatePayloadToDomain(t *testing.T) { UID: "member-123", MailingListUID: "ml-456", Email: "existing@example.com", - Username: "", // CLEARED (PUT semantics) - FirstName: "Updated", // Updated - LastName: "Name", // Updated - Organization: "", // CLEARED (PUT semantics) - JobTitle: "", // CLEARED (PUT semantics) - DeliveryMode: "", // CLEARED (PUT semantics) - ModStatus: "", // CLEARED (PUT semantics) - MemberType: "direct", // IMMUTABLE - Status: "active", // IMMUTABLE + Username: "", // CLEARED (PUT semantics) + FirstName: "Updated", // Updated + LastName: "Name", // Updated + Organization: "", // CLEARED (PUT semantics) + JobTitle: "", // CLEARED (PUT semantics) + DeliveryMode: "", // CLEARED (PUT semantics) + ModStatus: "", // CLEARED (PUT semantics) + MemberType: "direct", // IMMUTABLE + Status: "active", // IMMUTABLE CreatedAt: existingMember.CreatedAt, }, }, @@ -394,15 +394,15 @@ func TestConvertMemberUpdatePayloadToDomain(t *testing.T) { UID: "member-123", MailingListUID: "ml-456", Email: "existing@example.com", - Username: "", // CLEARED (PUT semantics) - FirstName: "", // CLEARED (PUT semantics) - LastName: "", // CLEARED (PUT semantics) - Organization: "", // CLEARED (PUT semantics) - JobTitle: "", // CLEARED (PUT semantics) - DeliveryMode: "normal", // Updated - ModStatus: "", // CLEARED (PUT semantics) - MemberType: "direct", // IMMUTABLE - Status: "active", // IMMUTABLE + Username: "", // CLEARED (PUT semantics) + FirstName: "", // CLEARED (PUT semantics) + LastName: "", // CLEARED (PUT semantics) + Organization: "", // CLEARED (PUT semantics) + JobTitle: "", // CLEARED (PUT semantics) + DeliveryMode: "normal", // Updated + ModStatus: "", // CLEARED (PUT semantics) + MemberType: "direct", // IMMUTABLE + Status: "active", // IMMUTABLE CreatedAt: existingMember.CreatedAt, }, }, @@ -421,38 +421,38 @@ func TestConvertMemberUpdatePayloadToDomain(t *testing.T) { expected: &model.GrpsIOMember{ UID: "member-123", MailingListUID: "ml-456", - Email: "existing@example.com", // Immutable - Username: "newuser", // Updated - FirstName: "New", // Updated - LastName: "Person", // Updated - Organization: "New Corp", // Updated - JobTitle: "New Role", // Updated - DeliveryMode: "none", // Updated - ModStatus: "owner", // Updated - MemberType: "direct", // Immutable - Status: "active", // Immutable + Email: "existing@example.com", // Immutable + Username: "newuser", // Updated + FirstName: "New", // Updated + LastName: "Person", // Updated + Organization: "New Corp", // Updated + JobTitle: "New Role", // Updated + DeliveryMode: "none", // Updated + ModStatus: "owner", // Updated + MemberType: "direct", // Immutable + Status: "active", // Immutable CreatedAt: existingMember.CreatedAt, // Immutable }, }, { name: "empty update - no fields provided, all mutable fields cleared (PUT semantics)", existing: existingMember, - payload: &mailinglistservice.UpdateGrpsioMailingListMemberPayload{ + payload: &mailinglistservice.UpdateGrpsioMailingListMemberPayload{ // All fields nil - PUT semantics clears all mutable fields }, expected: &model.GrpsIOMember{ UID: "member-123", MailingListUID: "ml-456", Email: "existing@example.com", - Username: "", // CLEARED (PUT semantics) - FirstName: "", // CLEARED (PUT semantics) - LastName: "", // CLEARED (PUT semantics) - Organization: "", // CLEARED (PUT semantics) - JobTitle: "", // CLEARED (PUT semantics) - DeliveryMode: "", // CLEARED (PUT semantics) - ModStatus: "", // CLEARED (PUT semantics) - MemberType: "direct", // IMMUTABLE - Status: "active", // IMMUTABLE + Username: "", // CLEARED (PUT semantics) + FirstName: "", // CLEARED (PUT semantics) + LastName: "", // CLEARED (PUT semantics) + Organization: "", // CLEARED (PUT semantics) + JobTitle: "", // CLEARED (PUT semantics) + DeliveryMode: "", // CLEARED (PUT semantics) + ModStatus: "", // CLEARED (PUT semantics) + MemberType: "direct", // IMMUTABLE + Status: "active", // IMMUTABLE CreatedAt: existingMember.CreatedAt, }, }, @@ -488,16 +488,16 @@ func TestConvertMemberUpdatePayloadToDomain(t *testing.T) { func TestConvertServiceUpdatePayloadToDomain(t *testing.T) { now := time.Now().UTC() existingService := &model.GrpsIOService{ - UID: "service-123", - Type: "primary", - Domain: "existing.domain.com", - GroupID: 12345, - Status: "active", - Public: true, // Existing setting + UID: "service-123", + Type: "primary", + Domain: "existing.domain.com", + GroupID: int64Ptr(12345), + Status: "active", + Public: true, // Existing setting GlobalOwners: []string{"existing@example.com"}, - ProjectUID: "project-123", - CreatedAt: now.Add(-24 * time.Hour), - UpdatedAt: now.Add(-1 * time.Hour), + ProjectUID: "project-123", + CreatedAt: now.Add(-24 * time.Hour), + UpdatedAt: now.Add(-1 * time.Hour), } tests := []struct { @@ -516,13 +516,13 @@ func TestConvertServiceUpdatePayloadToDomain(t *testing.T) { }, expected: &model.GrpsIOService{ UID: "service-123", - Type: "primary", // IMMUTABLE - Domain: "existing.domain.com", // IMMUTABLE - GroupID: 12345, // IMMUTABLE - Status: "updated", // Updated - Public: false, // CLEARED (PUT semantics) - GlobalOwners: nil, // CLEARED (PUT semantics) - ProjectUID: "project-123", // IMMUTABLE + Type: "primary", // IMMUTABLE + Domain: "existing.domain.com", // IMMUTABLE + GroupID: int64Ptr(12345), // IMMUTABLE + Status: "updated", // Updated + Public: false, // CLEARED (PUT semantics) + GlobalOwners: nil, // CLEARED (PUT semantics) + ProjectUID: "project-123", // IMMUTABLE CreatedAt: existingService.CreatedAt, // IMMUTABLE }, }, @@ -536,13 +536,13 @@ func TestConvertServiceUpdatePayloadToDomain(t *testing.T) { }, expected: &model.GrpsIOService{ UID: "service-123", - Type: "primary", // IMMUTABLE - Domain: "existing.domain.com", // IMMUTABLE - GroupID: 12345, // IMMUTABLE - Status: "", // CLEARED (PUT semantics) - Public: false, // Updated - GlobalOwners: nil, // CLEARED (PUT semantics) - ProjectUID: "project-123", // IMMUTABLE + Type: "primary", // IMMUTABLE + Domain: "existing.domain.com", // IMMUTABLE + GroupID: int64Ptr(12345), // IMMUTABLE + Status: "", // CLEARED (PUT semantics) + Public: false, // Updated + GlobalOwners: nil, // CLEARED (PUT semantics) + ProjectUID: "project-123", // IMMUTABLE CreatedAt: existingService.CreatedAt, // IMMUTABLE }, }, @@ -559,14 +559,14 @@ func TestConvertServiceUpdatePayloadToDomain(t *testing.T) { }, expected: &model.GrpsIOService{ UID: "service-123", - Type: "primary", // IMMUTABLE (can't change service type) - Domain: "existing.domain.com", // IMMUTABLE - GroupID: 12345, // IMMUTABLE - Status: "disabled", // Updated - Public: false, // Updated + Type: "primary", // IMMUTABLE (can't change service type) + Domain: "existing.domain.com", // IMMUTABLE + GroupID: int64Ptr(12345), // IMMUTABLE + Status: "disabled", // Updated + Public: false, // Updated GlobalOwners: []string{"new1@example.com", "new2@example.com"}, // Updated - ProjectUID: "project-123", // IMMUTABLE (can't change project) - CreatedAt: existingService.CreatedAt, // IMMUTABLE + ProjectUID: "project-123", // IMMUTABLE (can't change project) + CreatedAt: existingService.CreatedAt, // IMMUTABLE }, }, { @@ -578,13 +578,13 @@ func TestConvertServiceUpdatePayloadToDomain(t *testing.T) { }, expected: &model.GrpsIOService{ UID: "service-123", - Type: "primary", // IMMUTABLE - Domain: "existing.domain.com", // IMMUTABLE - GroupID: 12345, // IMMUTABLE - Status: "", // CLEARED (PUT semantics) - Public: false, // CLEARED to default false (PUT semantics) - GlobalOwners: nil, // CLEARED (PUT semantics) - ProjectUID: "project-123", // IMMUTABLE + Type: "primary", // IMMUTABLE + Domain: "existing.domain.com", // IMMUTABLE + GroupID: int64Ptr(12345), // IMMUTABLE + Status: "", // CLEARED (PUT semantics) + Public: false, // CLEARED to default false (PUT semantics) + GlobalOwners: nil, // CLEARED (PUT semantics) + ProjectUID: "project-123", // IMMUTABLE CreatedAt: existingService.CreatedAt, // IMMUTABLE }, }, @@ -618,7 +618,7 @@ func TestConvertMailingListUpdatePayloadToDomain(t *testing.T) { existingMailingList := &model.GrpsIOMailingList{ UID: "ml-123", GroupName: "existing-group", - Public: false, // Existing setting + Public: false, // Existing setting Type: "discussion_moderated", Description: "Existing description for the group", Title: "Existing Title", @@ -642,14 +642,14 @@ func TestConvertMailingListUpdatePayloadToDomain(t *testing.T) { // All other fields nil - PUT semantics clears mutable fields }, expected: &model.GrpsIOMailingList{ - UID: "ml-123", // IMMUTABLE - GroupName: "existing-group", // IMMUTABLE - Public: false, // CLEARED to default false - Type: "", // CLEARED (PUT semantics) - Description: "", // CLEARED (PUT semantics) - Title: "Updated Title", // Updated - ServiceUID: "", // CLEARED (PUT semantics) - ProjectUID: "project-123", // IMMUTABLE + UID: "ml-123", // IMMUTABLE + GroupName: "existing-group", // IMMUTABLE + Public: false, // CLEARED to default false + Type: "", // CLEARED (PUT semantics) + Description: "", // CLEARED (PUT semantics) + Title: "Updated Title", // Updated + ServiceUID: "", // CLEARED (PUT semantics) + ProjectUID: "project-123", // IMMUTABLE CreatedAt: existingMailingList.CreatedAt, // IMMUTABLE }, }, @@ -661,14 +661,14 @@ func TestConvertMailingListUpdatePayloadToDomain(t *testing.T) { // All other fields nil - PUT semantics clears mutable fields }, expected: &model.GrpsIOMailingList{ - UID: "ml-123", // IMMUTABLE - GroupName: "existing-group", // IMMUTABLE - Public: true, // Updated - Type: "", // CLEARED (PUT semantics) - Description: "", // CLEARED (PUT semantics) - Title: "", // CLEARED (PUT semantics) - ServiceUID: "", // CLEARED (PUT semantics) - ProjectUID: "project-123", // IMMUTABLE + UID: "ml-123", // IMMUTABLE + GroupName: "existing-group", // IMMUTABLE + Public: true, // Updated + Type: "", // CLEARED (PUT semantics) + Description: "", // CLEARED (PUT semantics) + Title: "", // CLEARED (PUT semantics) + ServiceUID: "", // CLEARED (PUT semantics) + ProjectUID: "project-123", // IMMUTABLE CreatedAt: existingMailingList.CreatedAt, // IMMUTABLE }, }, @@ -684,32 +684,32 @@ func TestConvertMailingListUpdatePayloadToDomain(t *testing.T) { ServiceUID: "new-service-456", }, expected: &model.GrpsIOMailingList{ - UID: "ml-123", // IMMUTABLE - GroupName: "existing-group", // IMMUTABLE (can't change group name) - Public: true, // Updated - Type: "discussion_open", // Updated + UID: "ml-123", // IMMUTABLE + GroupName: "existing-group", // IMMUTABLE (can't change group name) + Public: true, // Updated + Type: "discussion_open", // Updated Description: "New description that is long enough", // Updated - Title: "New Title", // Updated - ServiceUID: "new-service-456", // Updated - ProjectUID: "project-123", // IMMUTABLE - CreatedAt: existingMailingList.CreatedAt, // PRESERVED (immutable) + Title: "New Title", // Updated + ServiceUID: "new-service-456", // Updated + ProjectUID: "project-123", // IMMUTABLE + CreatedAt: existingMailingList.CreatedAt, // PRESERVED (immutable) }, }, { name: "empty update - no fields provided, all mutable fields cleared (PUT semantics)", existing: existingMailingList, - payload: &mailinglistservice.UpdateGrpsioMailingListPayload{ + payload: &mailinglistservice.UpdateGrpsioMailingListPayload{ // All fields nil - PUT semantics clears all mutable fields }, expected: &model.GrpsIOMailingList{ - UID: "ml-123", // IMMUTABLE - GroupName: "existing-group", // IMMUTABLE - Public: false, // CLEARED to default false - Type: "", // CLEARED (PUT semantics) - Description: "", // CLEARED (PUT semantics) - Title: "", // CLEARED (PUT semantics) - ServiceUID: "", // CLEARED (PUT semantics) - ProjectUID: "project-123", // IMMUTABLE + UID: "ml-123", // IMMUTABLE + GroupName: "existing-group", // IMMUTABLE + Public: false, // CLEARED to default false + Type: "", // CLEARED (PUT semantics) + Description: "", // CLEARED (PUT semantics) + Title: "", // CLEARED (PUT semantics) + ServiceUID: "", // CLEARED (PUT semantics) + ProjectUID: "project-123", // IMMUTABLE CreatedAt: existingMailingList.CreatedAt, // IMMUTABLE }, }, @@ -738,10 +738,6 @@ func TestConvertMailingListUpdatePayloadToDomain(t *testing.T) { } } -func boolPtr(b bool) *bool { - return &b -} - // Helper functions for creating pointers to primitives func stringPtr(s string) *string { return &s diff --git a/cmd/mailing-list-api/service/service_response_converters.go b/cmd/mailing-list-api/service/service_response_converters.go index 8deb93c..ba4be17 100644 --- a/cmd/mailing-list-api/service/service_response_converters.go +++ b/cmd/mailing-list-api/service/service_response_converters.go @@ -21,7 +21,7 @@ func (s *mailingListService) convertGrpsIOServiceDomainToFullResponse(service *m UID: &service.UID, Type: service.Type, Domain: &service.Domain, - GroupID: &service.GroupID, + GroupID: service.GroupID, Status: &service.Status, GlobalOwners: service.GlobalOwners, Prefix: &service.Prefix, @@ -63,7 +63,7 @@ func (s *mailingListService) convertGrpsIOServiceDomainToStandardResponse(servic UID: &service.UID, Type: service.Type, Domain: &service.Domain, - GroupID: &service.GroupID, + GroupID: service.GroupID, Status: &service.Status, GlobalOwners: service.GlobalOwners, Prefix: &service.Prefix, @@ -197,11 +197,11 @@ func (s *mailingListService) convertGrpsIOMemberToResponse(member *model.GrpsIOM } // Handle optional GroupsIO fields - if member.GroupsIOMemberID > 0 { - result.GroupsioMemberID = &member.GroupsIOMemberID + if member.GroupsIOMemberID != nil && *member.GroupsIOMemberID > 0 { + result.GroupsioMemberID = member.GroupsIOMemberID } - if member.GroupsIOGroupID > 0 { - result.GroupsioGroupID = &member.GroupsIOGroupID + if member.GroupsIOGroupID != nil && *member.GroupsIOGroupID > 0 { + result.GroupsioGroupID = member.GroupsIOGroupID } // Handle timestamps @@ -249,11 +249,11 @@ func (s *mailingListService) convertGrpsIOMemberDomainToResponse(member *model.G if member.JobTitle != "" { response.JobTitle = &member.JobTitle } - if member.GroupsIOMemberID != 0 { - response.GroupsioMemberID = &member.GroupsIOMemberID + if member.GroupsIOMemberID != nil && *member.GroupsIOMemberID != 0 { + response.GroupsioMemberID = member.GroupsIOMemberID } - if member.GroupsIOGroupID != 0 { - response.GroupsioGroupID = &member.GroupsIOGroupID + if member.GroupsIOGroupID != nil && *member.GroupsIOGroupID != 0 { + response.GroupsioGroupID = member.GroupsIOGroupID } if member.LastReviewedAt != nil { response.LastReviewedAt = member.LastReviewedAt diff --git a/cmd/mailing-list-api/service/service_response_converters_test.go b/cmd/mailing-list-api/service/service_response_converters_test.go index 329dc64..8dd50da 100644 --- a/cmd/mailing-list-api/service/service_response_converters_test.go +++ b/cmd/mailing-list-api/service/service_response_converters_test.go @@ -27,7 +27,7 @@ func TestConvertDomainToFullResponse(t *testing.T) { UID: "service-123", Type: "primary", Domain: "example.groups.io", - GroupID: 12345, + GroupID: int64Ptr(12345), Status: "active", GlobalOwners: []string{"owner1@example.com", "owner2@example.com"}, Prefix: "", @@ -79,7 +79,7 @@ func TestConvertDomainToFullResponse(t *testing.T) { UID: stringPtr(""), Type: "formation", Domain: stringPtr(""), - GroupID: int64Ptr(0), + GroupID: nil, Status: stringPtr(""), GlobalOwners: nil, Prefix: stringPtr(""), @@ -130,7 +130,7 @@ func TestConvertDomainToStandardResponse(t *testing.T) { UID: "service-123", Type: "shared", Domain: "shared.groups.io", - GroupID: 67890, + GroupID: int64Ptr(67890), Status: "inactive", GlobalOwners: []string{"shared@example.com"}, Prefix: "shared-prefix", @@ -183,7 +183,7 @@ func TestConvertDomainToStandardResponse(t *testing.T) { UID: stringPtr("service-456"), Type: "formation", Domain: stringPtr(""), - GroupID: int64Ptr(0), + GroupID: nil, Status: stringPtr(""), GlobalOwners: nil, Prefix: stringPtr(""), diff --git a/cmd/mailing-list-api/service/service_validators.go b/cmd/mailing-list-api/service/service_validators.go index b45c33e..d4a14c0 100644 --- a/cmd/mailing-list-api/service/service_validators.go +++ b/cmd/mailing-list-api/service/service_validators.go @@ -176,8 +176,15 @@ func validateUpdateImmutabilityConstraints(existing *model.GrpsIOService, payloa } // Check group_id immutability - only validate if explicitly provided - if payload.GroupID != nil && *payload.GroupID != existing.GroupID { - return errors.NewValidation(fmt.Sprintf("field 'group_id' is immutable. Cannot change from '%d' to '%d'", existing.GroupID, *payload.GroupID)) + if payload.GroupID != nil { + // Compare nullable pointers properly + if existing.GroupID == nil || *payload.GroupID != *existing.GroupID { + existingVal := "null" + if existing.GroupID != nil { + existingVal = fmt.Sprintf("%d", *existing.GroupID) + } + return errors.NewValidation(fmt.Sprintf("field 'group_id' is immutable. Cannot change from '%s' to '%d'", existingVal, *payload.GroupID)) + } } // Check url immutability - only validate if explicitly provided diff --git a/cmd/mailing-list-api/service/service_validators_test.go b/cmd/mailing-list-api/service/service_validators_test.go index dc50f25..0ae71c9 100644 --- a/cmd/mailing-list-api/service/service_validators_test.go +++ b/cmd/mailing-list-api/service/service_validators_test.go @@ -367,7 +367,7 @@ func TestValidateUpdateImmutabilityConstraints(t *testing.T) { ProjectUID: "project-123", Prefix: "", Domain: "example.groups.io", - GroupID: 12345, + GroupID: int64Ptr(12345), URL: "https://example.groups.io/g/test", GroupName: "test-group", } diff --git a/go.mod b/go.mod index 95d1d2a..f1f9bc6 100644 --- a/go.mod +++ b/go.mod @@ -7,6 +7,8 @@ go 1.24.0 require ( github.com/auth0/go-jwt-middleware/v2 v2.3.0 + github.com/golang-jwt/jwt/v5 v5.3.0 + github.com/google/go-querystring v1.1.0 github.com/google/uuid v1.6.0 github.com/nats-io/nats.go v1.31.0 github.com/stretchr/testify v1.10.0 diff --git a/go.sum b/go.sum index 4bdda97..67a1424 100644 --- a/go.sum +++ b/go.sum @@ -16,10 +16,15 @@ github.com/go-logr/stdr v1.2.2 h1:hSWxHoqTgW2S2qGc0LTAI563KZ5YKYRhT3MFKZMbjag= github.com/go-logr/stdr v1.2.2/go.mod h1:mMo/vtBO5dYbehREoey6XUKy/eSumjCCveDpRre4VKE= github.com/gohugoio/hashstructure v0.5.0 h1:G2fjSBU36RdwEJBWJ+919ERvOVqAg9tfcYp47K9swqg= github.com/gohugoio/hashstructure v0.5.0/go.mod h1:Ser0TniXuu/eauYmrwM4o64EBvySxNzITEOLlm4igec= +github.com/golang-jwt/jwt/v5 v5.3.0 h1:pv4AsKCKKZuqlgs5sUmn4x8UlGa0kEVt/puTpKx9vvo= +github.com/golang-jwt/jwt/v5 v5.3.0/go.mod h1:fxCRLWMO43lRc8nhHWY6LGqRcf+1gQWArsqaEUEa5bE= github.com/golang/protobuf v1.5.4 h1:i7eJL8qZTpSEXOPTxNKhASYpMn+8e5Q6AdndVa1dWek= github.com/golang/protobuf v1.5.4/go.mod h1:lnTiLA8Wa4RWRcIUkrtSVa5nRhsEGBg48fD6rSs7xps= +github.com/google/go-cmp v0.5.2/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8= github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU= +github.com/google/go-querystring v1.1.0 h1:AnCroh3fv4ZBgVIf1Iwtovgjaw/GiKJo8M8yD/fhyJ8= +github.com/google/go-querystring v1.1.0/go.mod h1:Kcdr2DB4koayq7X8pmAG4sNG59So17icRSOU623lUBU= github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/gorilla/websocket v1.5.3 h1:saDtZ6Pbx/0u+bgYQ3q96pZgCzfhKXGPqt7kZ72aNNg= @@ -82,6 +87,7 @@ golang.org/x/text v0.27.0 h1:4fGWRpyh641NLlecmyl4LOe6yDdfaYNrGb2zdfo4JV4= golang.org/x/text v0.27.0/go.mod h1:1D28KMCvyooCX9hBiosv5Tz/+YLxj0j7XhWjpSUF7CU= golang.org/x/tools v0.35.0 h1:mBffYraMEf7aa0sB+NuKnuCy8qI/9Bughn8dC2Gu5r0= golang.org/x/tools v0.35.0/go.mod h1:NKdj5HkL/73byiZSJjqJgKn3ep7KjFkBOkR/Hps3VPw= +golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= google.golang.org/genproto/googleapis/rpc v0.0.0-20250721164621-a45f3dfb1074 h1:qJW29YvkiJmXOYMu5Tf8lyrTp3dOS+K4z6IixtLaCf8= google.golang.org/genproto/googleapis/rpc v0.0.0-20250721164621-a45f3dfb1074/go.mod h1:qQ0YXyHHx3XkvlzUtpXDkS29lDSafHMZBAZDc03LQ3A= google.golang.org/grpc v1.74.2 h1:WoosgB65DlWVC9FqI82dGsZhWFNBSLjQ84bjROOpMu4= diff --git a/internal/domain/model/grpsio_mailing_list.go b/internal/domain/model/grpsio_mailing_list.go index ecb91a4..556406c 100644 --- a/internal/domain/model/grpsio_mailing_list.go +++ b/internal/domain/model/grpsio_mailing_list.go @@ -18,15 +18,24 @@ import ( ) // GrpsIOMailingList represents a GroupsIO mailing list entity with committee support +// Sync status constants for GroupsIO integration +const ( + SyncStatusPending = "pending" // Initial state, waiting for Groups.io sync + SyncStatusSynced = "synced" // Successfully synced with Groups.io + SyncStatusFailed = "failed" // Sync failed, requires reconciliation +) + type GrpsIOMailingList struct { UID string `json:"uid"` + SubgroupID *int64 `json:"-"` // Groups.io subgroup ID - internal use only, nullable for async GroupName string `json:"group_name"` - Public bool `json:"public"` // Whether the mailing list is publicly accessible - Type string `json:"type"` // "announcement" | "discussion_moderated" | "discussion_open" - CommitteeUID string `json:"committee_uid"` // Committee UUID (optional) - CommitteeName string `json:"committee_name"` // Committee name (optional) - CommitteeFilters []string `json:"committee_filters"` // Committee member filters (optional) - Description string `json:"description"` // Minimum 11 characters + Public bool `json:"public"` // Whether the mailing list is publicly accessible + SyncStatus string `json:"sync_status,omitempty"` // "pending", "synced", "failed" + Type string `json:"type"` // "announcement" | "discussion_moderated" | "discussion_open" + CommitteeUID string `json:"committee_uid"` // Committee UUID (optional) + CommitteeName string `json:"committee_name"` // Committee name (optional) + CommitteeFilters []string `json:"committee_filters"` // Committee member filters (optional) + Description string `json:"description"` // Minimum 11 characters Title string `json:"title"` SubjectTag string `json:"subject_tag"` // Optional ServiceUID string `json:"service_uid"` // Service UUID (required) diff --git a/internal/domain/model/grpsio_member.go b/internal/domain/model/grpsio_member.go index e4eff19..8492244 100644 --- a/internal/domain/model/grpsio_member.go +++ b/internal/domain/model/grpsio_member.go @@ -23,9 +23,10 @@ type GrpsIOMember struct { UID string `json:"uid"` // Primary key MailingListUID string `json:"mailing_list_uid"` // FK to mailing list - // External Groups.io IDs - GroupsIOMemberID int64 `json:"groupsio_member_id"` // From Groups.io - GroupsIOGroupID int64 `json:"groupsio_group_id"` // From Groups.io + // External Groups.io IDs (nullable for async support, hidden from API) + GroupsIOMemberID *int64 `json:"-"` // Groups.io member ID - internal use only + GroupsIOGroupID *int64 `json:"-"` // Groups.io subgroup ID - internal use only + SyncStatus string `json:"sync_status,omitempty"` // "pending", "synced", "failed" // Member Information Username string `json:"username"` // Username diff --git a/internal/domain/model/grpsio_member_test.go b/internal/domain/model/grpsio_member_test.go index 48e473e..463841c 100644 --- a/internal/domain/model/grpsio_member_test.go +++ b/internal/domain/model/grpsio_member_test.go @@ -25,8 +25,8 @@ func TestGrpsIOMember_BuildIndexKey(t *testing.T) { member: &GrpsIOMember{ UID: uuid.New().String(), MailingListUID: "mailing-list-123", - GroupsIOMemberID: 12345, - GroupsIOGroupID: 67890, + GroupsIOMemberID: func() *int64 { id := int64(12345); return &id }(), + GroupsIOGroupID: func() *int64 { id := int64(67890); return &id }(), Username: "testuser", FirstName: "John", LastName: "Doe", @@ -350,8 +350,8 @@ func TestGrpsIOMember_ValidationScenarios(t *testing.T) { member := &GrpsIOMember{ UID: uuid.New().String(), MailingListUID: "committee-mailing-list", - GroupsIOMemberID: 12345, - GroupsIOGroupID: 67890, + GroupsIOMemberID: int64Ptr(12345), + GroupsIOGroupID: int64Ptr(67890), Username: "committee-member", FirstName: "Committee", LastName: "Member", diff --git a/internal/domain/model/grpsio_service.go b/internal/domain/model/grpsio_service.go index 2c41b9e..4ec0175 100644 --- a/internal/domain/model/grpsio_service.go +++ b/internal/domain/model/grpsio_service.go @@ -15,13 +15,17 @@ import ( "github.com/linuxfoundation/lfx-v2-mailing-list-service/pkg/utils" ) +// DefaultGroupsIODomain is the default domain for Groups.io API calls +const DefaultGroupsIODomain = "groups.io" + // GrpsIOService represents a GroupsIO service entity type GrpsIOService struct { Type string `json:"type"` UID string `json:"uid"` Domain string `json:"domain"` - GroupID int64 `json:"group_id"` + GroupID *int64 `json:"-"` // Groups.io group ID - internal use only, nullable for async Status string `json:"status"` + SyncStatus string `json:"sync_status,omitempty"` // "pending", "synced", "failed" GlobalOwners []string `json:"global_owners"` Prefix string `json:"prefix"` ProjectSlug string `json:"project_slug"` @@ -53,8 +57,8 @@ func (s *GrpsIOService) BuildIndexKey(ctx context.Context) string { // Formation service: unique by project + prefix data = fmt.Sprintf("%s|%s|%s", s.ProjectUID, s.Type, s.Prefix) case "shared": - // Shared service: unique by project + group_id - data = fmt.Sprintf("%s|%s|%d", s.ProjectUID, s.Type, s.GroupID) + // Shared service: unique by project + group_name (decoupled from GroupID) + data = fmt.Sprintf("%s|%s|%s", s.ProjectUID, s.Type, s.GroupName) default: // Fallback for unknown types data = fmt.Sprintf("%s|%s|%s", s.ProjectUID, s.Type, s.UID) @@ -67,7 +71,7 @@ func (s *GrpsIOService) BuildIndexKey(ctx context.Context) string { "project_uid", s.ProjectUID, "service_type", s.Type, "service_prefix", s.Prefix, - "service_group_id", s.GroupID, + "service_group_name", s.GroupName, "key", key, ) @@ -117,3 +121,29 @@ func (s *GrpsIOService) ValidateLastReviewedAt() error { func (s *GrpsIOService) GetLastReviewedAtTime() (*time.Time, error) { return utils.ParseTimestampPtr(s.LastReviewedAt) } + +// GetDomain returns the appropriate domain for Groups.io API calls +func (s *GrpsIOService) GetDomain() string { + if s.Domain != "" { + return s.Domain // Use custom domain if set + } + return DefaultGroupsIODomain // Default to groups.io +} + +// GetGroupName returns the appropriate group name for Groups.io API calls with comprehensive fallback logic +func (s *GrpsIOService) GetGroupName() string { + if s.GroupName != "" { + return s.GroupName // Use explicit group name if set + } + + switch s.Type { + case "primary": + return s.ProjectSlug + case "formation": + return fmt.Sprintf("%s-formation", s.ProjectSlug) + case "shared": + return s.ProjectSlug // fallback for shared services + default: + return s.ProjectUID // fallback for unknown types + } +} diff --git a/internal/domain/model/grpsio_service_test.go b/internal/domain/model/grpsio_service_test.go index 4f483ef..a122246 100644 --- a/internal/domain/model/grpsio_service_test.go +++ b/internal/domain/model/grpsio_service_test.go @@ -11,6 +11,9 @@ import ( "github.com/stretchr/testify/assert" ) +// Helper function for test pointer creation +func int64Ptr(v int64) *int64 { return &v } + func TestGrpsIOService_BuildIndexKey(t *testing.T) { ctx := context.Background() @@ -26,7 +29,7 @@ func TestGrpsIOService_BuildIndexKey(t *testing.T) { UID: "service-123", ProjectUID: "project-456", Prefix: "test-prefix", - GroupID: 12345, + GroupID: int64Ptr(12345), }, description: "Primary service should use project_uid and type only", }, @@ -37,7 +40,7 @@ func TestGrpsIOService_BuildIndexKey(t *testing.T) { UID: "service-456", ProjectUID: "project-789", Prefix: "formation-prefix", - GroupID: 67890, + GroupID: int64Ptr(67890), }, description: "Formation service should use project_uid, type, and prefix", }, @@ -48,7 +51,7 @@ func TestGrpsIOService_BuildIndexKey(t *testing.T) { UID: "service-789", ProjectUID: "project-123", Prefix: "shared-prefix", - GroupID: 54321, + GroupID: int64Ptr(54321), }, description: "Shared service should use project_uid, type, and group_id", }, @@ -59,7 +62,7 @@ func TestGrpsIOService_BuildIndexKey(t *testing.T) { UID: "service-999", ProjectUID: "project-999", Prefix: "unknown-prefix", - GroupID: 99999, + GroupID: int64Ptr(99999), }, description: "Unknown service type should use project_uid, type, and uid as fallback", }, @@ -117,7 +120,7 @@ func TestGrpsIOService_BuildIndexKey(t *testing.T) { sharedService := &GrpsIOService{ Type: "shared", ProjectUID: projectUID, - GroupID: 12345, + GroupID: int64Ptr(12345), } primaryKey := primaryService.BuildIndexKey(ctx) @@ -157,18 +160,20 @@ func TestGrpsIOService_BuildIndexKey(t *testing.T) { sharedService1 := &GrpsIOService{ Type: "shared", ProjectUID: projectUID, - GroupID: 12345, + GroupName: "group-1", + GroupID: int64Ptr(12345), } sharedService2 := &GrpsIOService{ Type: "shared", ProjectUID: projectUID, - GroupID: 67890, + GroupName: "group-2", + GroupID: int64Ptr(67890), } key1 := sharedService1.BuildIndexKey(ctx) key2 := sharedService2.BuildIndexKey(ctx) - assert.NotEqual(t, key1, key2, "Shared services with different group IDs should have different keys") + assert.NotEqual(t, key1, key2, "Shared services with different group names should have different keys") }) } @@ -331,7 +336,7 @@ func BenchmarkGrpsIOService_BuildIndexKey(b *testing.B) { service: &GrpsIOService{ Type: "shared", ProjectUID: "project-" + uuid.New().String(), - GroupID: 12345, + GroupID: int64Ptr(12345), }, }, } diff --git a/internal/infrastructure/groupsio/client.go b/internal/infrastructure/groupsio/client.go new file mode 100644 index 0000000..3bd0a3b --- /dev/null +++ b/internal/infrastructure/groupsio/client.go @@ -0,0 +1,502 @@ +// Copyright The Linux Foundation and each contributor to LFX. +// SPDX-License-Identifier: MIT + +package groupsio + +import ( + "context" + "encoding/json" + "fmt" + "io" + "log/slog" + "net/http" + "net/url" + "strconv" + "strings" + "sync" + "time" + + "github.com/golang-jwt/jwt/v5" + "github.com/google/go-querystring/query" + "github.com/linuxfoundation/lfx-v2-mailing-list-service/pkg/httpclient" +) + +// groupsioBasicAuthRoundTripper implements automatic BasicAuth injection (production pattern) +type groupsioBasicAuthRoundTripper struct { + client *Client +} + +// RoundTrip ensures authentication before non-login requests to groups.io +// and adds authorization header to those requests (production pattern) +func (rt *groupsioBasicAuthRoundTripper) RoundTrip(req *http.Request, next func(*http.Request) (*http.Response, error)) (*http.Response, error) { + // Skip auth for login requests to avoid infinite recursion + if strings.Contains(req.URL.Path, "/v1/login") { + return next(req) + } + + // Get cached token for this domain + rt.client.cacheMu.RLock() + if cache, exists := rt.client.cache[req.Host]; exists { + cache.mu.RLock() + if time.Now().Before(cache.expiry) && cache.token != "" { + // Use cached token with BasicAuth (production pattern: req.SetBasicAuth) + req.SetBasicAuth(cache.token, "") + cache.mu.RUnlock() + rt.client.cacheMu.RUnlock() + + slog.DebugContext(req.Context(), "RoundTripper: using cached Groups.io token", + "host", req.Host, "path", req.URL.Path) + return next(req) + } + cache.mu.RUnlock() + } + rt.client.cacheMu.RUnlock() + + // No valid cached token - authenticate first (production pattern) + if err := rt.client.WithDomain(req.Context(), req.Host); err != nil { + return nil, fmt.Errorf("authentication failed in RoundTripper: %w", err) + } + + // Now get the token and set BasicAuth + rt.client.cacheMu.RLock() + if cache, exists := rt.client.cache[req.Host]; exists { + cache.mu.RLock() + if cache.token != "" { + req.SetBasicAuth(cache.token, "") + slog.DebugContext(req.Context(), "RoundTripper: using fresh Groups.io token", + "host", req.Host, "path", req.URL.Path) + } + cache.mu.RUnlock() + } + rt.client.cacheMu.RUnlock() + + return next(req) +} + +// tokenCache holds cached authentication tokens per domain +type tokenCache struct { + token string + expiry time.Time + mu sync.RWMutex +} + +// Client handles all Groups.io API operations with smart token caching +// ClientInterface defines the contract for GroupsIO API operations +type ClientInterface interface { + CreateGroup(ctx context.Context, domain string, options GroupCreateOptions) (*GroupObject, error) + DeleteGroup(ctx context.Context, domain string, groupID uint64) error + CreateSubgroup(ctx context.Context, domain string, parentGroupID uint64, options SubgroupCreateOptions) (*SubgroupObject, error) + DeleteSubgroup(ctx context.Context, domain string, subgroupID uint64) error + AddMember(ctx context.Context, domain string, subgroupID uint64, email, name string) (*MemberObject, error) + UpdateMember(ctx context.Context, domain string, memberID uint64, updates MemberUpdateOptions) error + UpdateGroup(ctx context.Context, domain string, groupID uint64, updates GroupUpdateOptions) error + UpdateSubgroup(ctx context.Context, domain string, subgroupID uint64, updates SubgroupUpdateOptions) error + RemoveMember(ctx context.Context, domain string, memberID uint64) error + IsReady(ctx context.Context) error +} + +type Client struct { + config Config + httpClient *httpclient.Client + cache map[string]*tokenCache // domain -> token + cacheMu sync.RWMutex +} + +// NewClient creates a new GroupsIO client with the given configuration +func NewClient(cfg Config) (*Client, error) { + if cfg.MockMode { + return nil, nil // Return nil for mock mode - orchestrator will handle this + } + + // Validate required configuration + if cfg.Email == "" || cfg.Password == "" { + return nil, fmt.Errorf("email and password are required for Groups.io client") + } + + if cfg.BaseURL == "" { + cfg.BaseURL = "https://api.groups.io" + } + + // Use reusable httpclient from pkg + httpConfig := httpclient.Config{ + Timeout: cfg.Timeout, + MaxRetries: cfg.MaxRetries, + RetryDelay: cfg.RetryDelay, + RetryBackoff: true, + MaxDelay: 30 * time.Second, // Cap exponential backoff at 30s + } + + client := &Client{ + config: cfg, + httpClient: httpclient.NewClient(httpConfig), + cache: make(map[string]*tokenCache), + } + + // Register BasicAuth RoundTripper for automatic token injection (production pattern) + authRoundTripper := &groupsioBasicAuthRoundTripper{client: client} + client.httpClient.AddRoundTripper(authRoundTripper) + + slog.InfoContext(context.Background(), "Groups.io client initialized with RoundTripper auth pattern") + + return client, nil +} + +// CreateGroup creates a main group in Groups.io +func (c *Client) CreateGroup(ctx context.Context, domain string, options GroupCreateOptions) (*GroupObject, error) { + slog.InfoContext(ctx, "creating group in Groups.io", + "domain", domain, "group_name", options.GroupName) + + data, err := query.Values(options) + if err != nil { + return nil, fmt.Errorf("failed to encode options: %w", err) + } + + var response GroupObject + err = c.makeRequest(ctx, domain, http.MethodPost, "/creategroup", data, &response) + if err != nil { + return nil, err + } + + slog.InfoContext(ctx, "group created successfully in Groups.io", + "group_id", response.ID, "domain", domain) + + return &response, nil +} + +// DeleteGroup removes a group from Groups.io +func (c *Client) DeleteGroup(ctx context.Context, domain string, groupID uint64) error { + slog.InfoContext(ctx, "deleting group from Groups.io", + "domain", domain, "group_id", groupID) + + data := url.Values{ + "group_id": {strconv.FormatUint(groupID, 10)}, + } + + return c.makeRequest(ctx, domain, http.MethodPost, "/deletegroup", data, nil) +} + +// CreateSubgroup creates a subgroup (mailing list) in Groups.io +func (c *Client) CreateSubgroup(ctx context.Context, domain string, parentGroupID uint64, options SubgroupCreateOptions) (*SubgroupObject, error) { + slog.InfoContext(ctx, "creating subgroup in Groups.io", + "domain", domain, "parent_group_id", parentGroupID, "subgroup_name", options.GroupName) + + // Set the parent group ID in options if not already set (for backwards compatibility) + if options.ParentGroupID == 0 { + options.ParentGroupID = parentGroupID + } + + data, err := query.Values(options) + if err != nil { + return nil, fmt.Errorf("failed to encode options: %w", err) + } + + var response SubgroupObject + err = c.makeRequest(ctx, domain, http.MethodPost, "/createsubgroup", data, &response) + if err != nil { + return nil, err + } + + slog.InfoContext(ctx, "subgroup created successfully in Groups.io", + "subgroup_id", response.ID, "parent_group_id", parentGroupID) + + return &response, nil +} + +// DeleteSubgroup removes a subgroup from Groups.io +func (c *Client) DeleteSubgroup(ctx context.Context, domain string, subgroupID uint64) error { + slog.InfoContext(ctx, "deleting subgroup from Groups.io", + "domain", domain, "subgroup_id", subgroupID) + + data := url.Values{ + "subgroup_id": {strconv.FormatUint(subgroupID, 10)}, + } + + return c.makeRequest(ctx, domain, http.MethodPost, "/deletesubgroup", data, nil) +} + +// AddMember adds a single member to a subgroup +func (c *Client) AddMember(ctx context.Context, domain string, subgroupID uint64, email, name string) (*MemberObject, error) { + slog.InfoContext(ctx, "adding member to Groups.io", + "domain", domain, "subgroup_id", subgroupID, "email", email) + + data := url.Values{ + "subgroup_id": {strconv.FormatUint(subgroupID, 10)}, + "email": {email}, + "name": {name}, + } + + var result MemberObject + err := c.makeRequest(ctx, domain, http.MethodPost, "/addmember", data, &result) + if err != nil { + return nil, err + } + + slog.InfoContext(ctx, "member added successfully to Groups.io", + "member_id", result.ID, "subgroup_id", subgroupID, "email", email) + + return &result, nil +} + +// UpdateMember updates member properties (e.g., mod_status) +func (c *Client) UpdateMember(ctx context.Context, domain string, memberID uint64, updates MemberUpdateOptions) error { + slog.InfoContext(ctx, "updating member in Groups.io", + "domain", domain, "member_id", memberID) + + data, err := query.Values(updates) + if err != nil { + return fmt.Errorf("failed to encode updates: %w", err) + } + data.Set("member_id", strconv.FormatUint(memberID, 10)) + + err = c.makeRequest(ctx, domain, http.MethodPost, "/updatemember", data, nil) + if err != nil { + return err + } + + slog.InfoContext(ctx, "member updated successfully in Groups.io", + "member_id", memberID) + + return nil +} + +// UpdateGroup updates a Groups.io group with the provided options +func (c *Client) UpdateGroup(ctx context.Context, domain string, groupID uint64, updates GroupUpdateOptions) error { + slog.InfoContext(ctx, "updating group in Groups.io", + "domain", domain, "group_id", groupID) + + data, err := query.Values(updates) + if err != nil { + return fmt.Errorf("failed to encode updates: %w", err) + } + data.Set("group_id", strconv.FormatUint(groupID, 10)) + + err = c.makeRequest(ctx, domain, http.MethodPost, "/updategroup", data, nil) + if err != nil { + return err + } + + slog.InfoContext(ctx, "group updated successfully in Groups.io", + "group_id", groupID) + + return nil +} + +// UpdateSubgroup updates a Groups.io subgroup with the provided options +func (c *Client) UpdateSubgroup(ctx context.Context, domain string, subgroupID uint64, updates SubgroupUpdateOptions) error { + slog.InfoContext(ctx, "updating subgroup in Groups.io", + "domain", domain, "subgroup_id", subgroupID) + + data, err := query.Values(updates) + if err != nil { + return fmt.Errorf("failed to encode updates: %w", err) + } + data.Set("subgroup_id", strconv.FormatUint(subgroupID, 10)) + + err = c.makeRequest(ctx, domain, http.MethodPost, "/updatesubgroup", data, nil) + if err != nil { + return err + } + + slog.InfoContext(ctx, "subgroup updated successfully in Groups.io", + "subgroup_id", subgroupID) + + return nil +} + +// RemoveMember removes a single member from a subgroup +func (c *Client) RemoveMember(ctx context.Context, domain string, memberID uint64) error { + slog.InfoContext(ctx, "removing member from Groups.io", + "domain", domain, "member_id", memberID) + + data := url.Values{ + "member_id": {strconv.FormatUint(memberID, 10)}, + } + + err := c.makeRequest(ctx, domain, http.MethodPost, "/removemember", data, nil) + if err != nil { + return err + } + + slog.InfoContext(ctx, "member removed successfully from Groups.io", + "member_id", memberID) + + return nil +} + +// makeRequest centralizes all API calls with authentication and error handling +func (c *Client) makeRequest(ctx context.Context, domain string, method string, path string, data url.Values, result interface{}) error { + // Get or refresh token for domain + token, err := c.getOrRefreshToken(ctx, domain) + if err != nil { + return fmt.Errorf("authentication failed: %w", err) + } + + // Build request URL + reqURL := c.config.BaseURL + "/v1" + path + + var body io.Reader + headers := map[string]string{} + + if method == "POST" && data != nil { + // Add cached token to POST data (Groups.io pattern) + data.Set("csrf", token) + body = strings.NewReader(data.Encode()) + headers["Content-Type"] = "application/x-www-form-urlencoded" + } else if method == "GET" && data != nil { + reqURL += "?" + data.Encode() + } + + // Set domain header for vhost auth (critical for multi-tenant) + headers["Host"] = domain + + // NOTE: BasicAuth is now handled automatically by RoundTripper (production pattern) + // No manual Authorization header needed - RoundTripper calls req.SetBasicAuth(token, "") + + // Make request using httpclient (with retry logic + RoundTripper auth) + resp, err := c.httpClient.Request(ctx, method, reqURL, body, headers) + if err != nil { + return MapHTTPError(ctx, err) + } + + // Parse response + if result != nil { + if err := json.Unmarshal(resp.Body, result); err != nil { + return fmt.Errorf("failed to parse response: %w", err) + } + } + + return nil +} + +// WithDomain ensures authentication for a domain (production pattern) +func (c *Client) WithDomain(ctx context.Context, domain string) error { + // Check if auth token is already cached + c.cacheMu.RLock() + if cache, exists := c.cache[domain]; exists { + cache.mu.RLock() + if time.Now().Before(cache.expiry) { + cache.mu.RUnlock() + c.cacheMu.RUnlock() + slog.DebugContext(ctx, "using cached Groups.io login token", "domain", domain) + return nil + } + cache.mu.RUnlock() + } + c.cacheMu.RUnlock() + + // Need to get new token + _, err := c.getToken(ctx, domain) + if err != nil { + slog.ErrorContext(ctx, "failed to authenticate with Groups.io", "error", err, "domain", domain) + return fmt.Errorf("failed to authenticate request: %w", err) + } + return nil +} + +// getOrRefreshToken implements smart token caching with JWT expiry +func (c *Client) getOrRefreshToken(ctx context.Context, domain string) (string, error) { + c.cacheMu.RLock() + if cache, exists := c.cache[domain]; exists { + cache.mu.RLock() + if time.Now().Before(cache.expiry) { + token := cache.token + cache.mu.RUnlock() + c.cacheMu.RUnlock() + return token, nil + } + cache.mu.RUnlock() + } + c.cacheMu.RUnlock() + + // Need to authenticate - use simplified getToken + return c.getToken(ctx, domain) +} + +// getToken authenticates a user and returns a login token (production pattern) +func (c *Client) getToken(ctx context.Context, domain string) (string, error) { + // Login endpoint with timeout (prevents hanging on invalid domains) + loginCtx, cancel := context.WithTimeout(ctx, c.config.Timeout) + defer cancel() + + data := url.Values{ + "email": {c.config.Email}, + "password": {c.config.Password}, + "token": {"true"}, + } + + // Set domain header for vhost auth (critical for multi-tenant) + headers := map[string]string{ + "Host": domain, + } + + // Use GET with query parameters (production pattern) + loginURL := c.config.BaseURL + "/v1/login?" + data.Encode() + resp, err := c.httpClient.Request(loginCtx, http.MethodGet, loginURL, nil, headers) + if err != nil { + return "", fmt.Errorf("login request failed: %w", MapHTTPError(loginCtx, err)) + } + + var loginResp LoginObject + if err := json.Unmarshal(resp.Body, &loginResp); err != nil { + return "", fmt.Errorf("login response parse failed: %w", err) + } + + token := loginResp.Token + if token == "" { + return "", fmt.Errorf("no token in login response") + } + + // Parse JWT for expiry (production pattern) + expiry := c.parseTokenExpiry(token) + + // Cache token for this domain + c.cacheMu.Lock() + if c.cache[domain] == nil { + c.cache[domain] = &tokenCache{} + } + cache := c.cache[domain] + c.cacheMu.Unlock() + + cache.mu.Lock() + cache.token = token + cache.expiry = expiry + cache.mu.Unlock() + + slog.InfoContext(ctx, "Groups.io authentication successful", + "domain", domain, "expires_at", expiry.Format(time.RFC3339)) + + return token, nil +} + +// parseTokenExpiry extracts expiry from JWT (reused from go-groupsio) +func (c *Client) parseTokenExpiry(token string) time.Time { + parser := jwt.Parser{} + claims := jwt.MapClaims{} + + _, _, err := parser.ParseUnverified(token, &claims) + if err != nil { + slog.Warn("failed to parse JWT token", "error", err) + return time.Now().Add(10 * time.Minute) // Default TTL + } + + exp, err := claims.GetExpirationTime() + if err != nil || exp == nil { + slog.Warn("no expiry in JWT token", "error", err) + return time.Now().Add(10 * time.Minute) // Default TTL + } + + // Cache until 1 minute before expiry + return exp.Time.Add(-1 * time.Minute) +} + +// IsReady checks if Groups.io API is accessible +func (c *Client) IsReady(ctx context.Context) error { + resp, err := c.httpClient.Request(ctx, "GET", c.config.BaseURL, nil, nil) + if err != nil { + return fmt.Errorf("groups.io API unreachable: %w", MapHTTPError(ctx, err)) + } + if resp.StatusCode >= 400 { + return fmt.Errorf("groups.io API unhealthy (status: %d)", resp.StatusCode) + } + return nil +} diff --git a/internal/infrastructure/groupsio/config.go b/internal/infrastructure/groupsio/config.go new file mode 100644 index 0000000..fb3fe53 --- /dev/null +++ b/internal/infrastructure/groupsio/config.go @@ -0,0 +1,87 @@ +// Copyright The Linux Foundation and each contributor to LFX. +// SPDX-License-Identifier: MIT + +package groupsio + +import ( + "os" + "strconv" + "time" +) + +// Config holds the configuration for the GroupsIO client +type Config struct { + // BaseURL is the Groups.io API base URL + BaseURL string + + // Email is the Groups.io account email for authentication + Email string + + // Password is the Groups.io account password for authentication + Password string + + // Timeout is the HTTP client timeout for requests + Timeout time.Duration + + // MaxRetries is the maximum number of retry attempts for failed requests + MaxRetries int + + // RetryDelay is the delay between retry attempts + RetryDelay time.Duration + + // MockMode disables real Groups.io API calls (for testing) + MockMode bool +} + +// DefaultConfig returns a Config with sensible defaults +func DefaultConfig() Config { + return Config{ + BaseURL: "https://api.groups.io", + Timeout: 30 * time.Second, + MaxRetries: 3, + RetryDelay: 1 * time.Second, + MockMode: false, + } +} + +// NewConfigFromEnv creates a Config from environment variables +func NewConfigFromEnv() Config { + config := DefaultConfig() + + if baseURL := os.Getenv("GROUPSIO_BASE_URL"); baseURL != "" { + config.BaseURL = baseURL + } + + if email := os.Getenv("GROUPSIO_EMAIL"); email != "" { + config.Email = email + } + + if password := os.Getenv("GROUPSIO_PASSWORD"); password != "" { + config.Password = password + } + + if timeoutStr := os.Getenv("GROUPSIO_TIMEOUT"); timeoutStr != "" { + if timeout, err := time.ParseDuration(timeoutStr); err == nil { + config.Timeout = timeout + } + } + + if retriesStr := os.Getenv("GROUPSIO_MAX_RETRIES"); retriesStr != "" { + if retries, err := strconv.Atoi(retriesStr); err == nil { + config.MaxRetries = retries + } + } + + if delayStr := os.Getenv("GROUPSIO_RETRY_DELAY"); delayStr != "" { + if delay, err := time.ParseDuration(delayStr); err == nil { + config.RetryDelay = delay + } + } + + // Check for mock mode + if mockMode := os.Getenv("GROUPSIO_SOURCE"); mockMode == "mock" { + config.MockMode = true + } + + return config +} diff --git a/internal/infrastructure/groupsio/errors.go b/internal/infrastructure/groupsio/errors.go new file mode 100644 index 0000000..0bb3388 --- /dev/null +++ b/internal/infrastructure/groupsio/errors.go @@ -0,0 +1,92 @@ +// Copyright The Linux Foundation and each contributor to LFX. +// SPDX-License-Identifier: MIT + +package groupsio + +import ( + "context" + "fmt" + "log/slog" + "net/http" + + "github.com/linuxfoundation/lfx-v2-mailing-list-service/pkg/errors" + "github.com/linuxfoundation/lfx-v2-mailing-list-service/pkg/httpclient" +) + +// MapHTTPError maps httpclient errors to domain errors with proper context logging +func MapHTTPError(ctx context.Context, err error) error { + if err == nil { + return nil + } + + // Check if it's a retryable error from httpclient + if retryableErr, ok := err.(*httpclient.RetryableError); ok { + slog.WarnContext(ctx, "Groups.io HTTP error occurred", + "status_code", retryableErr.StatusCode, + "message", retryableErr.Message, + ) + + switch retryableErr.StatusCode { + case http.StatusNotFound: + return errors.NewNotFound("resource not found in Groups.io", err) + case http.StatusConflict: + return errors.NewConflict("resource already exists in Groups.io", err) + case http.StatusUnauthorized: + return errors.NewUnauthorized("Groups.io authentication failed", err) + case http.StatusForbidden: + return errors.NewValidation("Groups.io access denied", err) + case http.StatusTooManyRequests: + return errors.NewServiceUnavailable("Groups.io rate limited", err) + case http.StatusBadRequest: + return errors.NewValidation(fmt.Sprintf("Groups.io validation error: %s", retryableErr.Message), err) + case http.StatusInternalServerError, http.StatusBadGateway, http.StatusServiceUnavailable, http.StatusGatewayTimeout: + return errors.NewServiceUnavailable("Groups.io service unavailable", err) + default: + slog.ErrorContext(ctx, "Unexpected Groups.io HTTP status code", + "status_code", retryableErr.StatusCode, + "message", retryableErr.Message, + ) + return errors.NewUnexpected("Groups.io API error", err) + } + } + + // Handle other error types (network, timeout, etc.) + slog.ErrorContext(ctx, "Groups.io request failed with non-HTTP error", + "error", err.Error(), + ) + return errors.NewUnexpected("Groups.io request failed", err) +} + +// WrapGroupsIOError wraps a Groups.io API error response with proper context logging +func WrapGroupsIOError(ctx context.Context, errObj *ErrorObject) error { + if errObj == nil { + return nil + } + + slog.WarnContext(ctx, "Groups.io API error response", + "error_type", errObj.Type, + "message", errObj.Message, + "code", errObj.Code, + ) + + switch errObj.Type { + case "validation_error": + return errors.NewValidation(errObj.Message) + case "not_found": + return errors.NewNotFound(errObj.Message) + case "conflict": + return errors.NewConflict(errObj.Message) + case "unauthorized": + return errors.NewUnauthorized(errObj.Message) + case "forbidden": + return errors.NewValidation(errObj.Message) // Access denied, not auth failure + case "rate_limited": + return errors.NewServiceUnavailable(errObj.Message) + default: + slog.ErrorContext(ctx, "Unknown Groups.io error type", + "error_type", errObj.Type, + "message", errObj.Message, + ) + return errors.NewUnexpected(errObj.Message) + } +} diff --git a/internal/infrastructure/groupsio/models.go b/internal/infrastructure/groupsio/models.go new file mode 100644 index 0000000..4ba2581 --- /dev/null +++ b/internal/infrastructure/groupsio/models.go @@ -0,0 +1,143 @@ +// Copyright The Linux Foundation and each contributor to LFX. +// SPDX-License-Identifier: MIT + +package groupsio + +import "time" + +// GroupObject represents a Groups.io group (expanded to match production structure) +type GroupObject struct { + ID uint64 `json:"id"` + Object string `json:"object"` + Created string `json:"created"` + Updated string `json:"updated"` + Title string `json:"title"` + Name string `json:"name"` + Alias string `json:"alias"` + Desc string `json:"desc"` + PlainDesc string `json:"plain_desc"` + SubjectTag string `json:"subject_tag"` + Footer string `json:"footer"` + Website string `json:"website"` + Announce bool `json:"announce"` + Restricted bool `json:"restricted"` + Moderated bool `json:"moderated"` + Privacy string `json:"privacy"` + OrgID uint64 `json:"org_id"` + OrgDomain string `json:"org_domain"` + SubsCount uint64 `json:"subs_count"` + GroupURL string `json:"group_url"` +} + +// SubgroupObject represents a Groups.io subgroup (mailing list) +type SubgroupObject struct { + ID uint64 `json:"id"` + GroupID uint64 `json:"group_id"` + Name string `json:"name"` + Description string `json:"description"` + Public bool `json:"public"` + Type string `json:"type"` // announcement, discussion_moderated, discussion_open + CreatedAt string `json:"created"` + UpdatedAt string `json:"updated"` +} + +// MemberObject represents a Groups.io member (simplified from go-groupsio) +type MemberObject struct { + ID uint64 `json:"id"` + GroupID uint64 `json:"group_id"` + SubgroupID uint64 `json:"subgroup_id,omitempty"` + Email string `json:"email"` + Name string `json:"name"` + FirstName string `json:"first_name,omitempty"` + LastName string `json:"last_name,omitempty"` + Username string `json:"username,omitempty"` + Status string `json:"status"` // normal, pending, bouncing, etc. + ModStatus string `json:"mod_status"` // none, moderator, owner + DeliveryMode string `json:"delivery"` // individual, digest, no_email + JoinedAt string `json:"joined"` + UpdatedAt string `json:"updated"` +} + +// LoginObject represents the Groups.io login response +type LoginObject struct { + Token string `json:"token"` + User interface{} `json:"user,omitempty"` // Can be object or array, we don't need to parse it + UserID uint64 `json:"user_id,omitempty"` + Email string `json:"email,omitempty"` + ExpiresAt string `json:"expires_at,omitempty"` +} + +// ErrorObject represents a Groups.io API error response +type ErrorObject struct { + Type string `json:"type"` + Message string `json:"message"` + Code int `json:"code,omitempty"` +} + +// GroupCreateOptions represents options for creating a group (matches production go-groupsio) +type GroupCreateOptions struct { + GroupName string `url:"group_name"` + Desc string `url:"desc"` + Privacy string `url:"privacy"` + SubGroupAccess string `url:"sub_group_access"` + + // Creator subscription options (from production) + EmailDelivery string `url:"email_delivery,omitempty"` + MessageSelection string `url:"message_selection,omitempty"` + AutoFollowReplies bool `url:"auto_follow_replies,omitempty"` +} + +// SubgroupCreateOptions represents options for creating a subgroup (matches production go-groupsio) +type SubgroupCreateOptions struct { + // Subgroup options (production field names) + ParentGroupID uint64 `url:"group_id,omitempty"` // Parent group ID + ParentGroupName string `url:"group_name,omitempty"` // Parent group name + GroupName string `url:"sub_group_name"` // REQUIRED by Groups.io API: must be "sub_group_name" not "subgroup_name" per API spec + Desc string `url:"desc"` // REQUIRED by Groups.io API: must be "desc" not "description" per API spec + Privacy string `url:"privacy,omitempty"` // Privacy setting (optional - may inherit from parent) + + // Creator subscription options (from production) + EmailDelivery string `url:"email_delivery,omitempty"` + MessageSelection string `url:"message_selection,omitempty"` + AutoFollowReplies bool `url:"auto_follow_replies,omitempty"` + MaxAttachmentSize string `url:"max_attachment_size,omitempty"` +} + +// MemberUpdateOptions represents options for updating a member +type MemberUpdateOptions struct { + ModStatus string `url:"mod_status,omitempty"` // none, moderator, owner + DeliveryMode string `url:"delivery,omitempty"` // individual, digest, no_email + Status string `url:"status,omitempty"` // normal, pending, bouncing + FirstName string `url:"first_name,omitempty"` + LastName string `url:"last_name,omitempty"` +} + +// GroupUpdateOptions represents options for updating a Groups.io group/service +type GroupUpdateOptions struct { + GlobalOwners []string `url:"global_owners,omitempty"` + Announce *bool `url:"announce,omitempty"` + ReplyTo string `url:"reply_to,omitempty"` + MembersVisible string `url:"members_visible,omitempty"` + CalendarAccess string `url:"calendar_access,omitempty"` + FilesAccess string `url:"files_access,omitempty"` + DatabaseAccess string `url:"database_access,omitempty"` + WikiAccess string `url:"wiki_access,omitempty"` + PhotosAccess string `url:"photos_access,omitempty"` + MemberDirectoryAccess string `url:"member_directory_access,omitempty"` + PollsAccess string `url:"polls_access,omitempty"` + ChatAccess string `url:"chat_access,omitempty"` +} + +// SubgroupUpdateOptions represents options for updating a Groups.io subgroup/mailing list +type SubgroupUpdateOptions struct { + Title string `url:"title,omitempty"` + Description string `url:"description,omitempty"` + SubjectTag string `url:"subject_tag,omitempty"` + Committee string `url:"committee,omitempty"` +} + +// TokenCache represents a cached authentication token +type TokenCache struct { + Token string + ExpiresAt time.Time +} diff --git a/internal/infrastructure/mock/error_simulation_test.go b/internal/infrastructure/mock/error_simulation_test.go new file mode 100644 index 0000000..f230157 --- /dev/null +++ b/internal/infrastructure/mock/error_simulation_test.go @@ -0,0 +1,156 @@ +// Copyright The Linux Foundation and each contributor to LFX. +// SPDX-License-Identifier: MIT + +package mock + +import ( + "context" + "errors" + "testing" + + "github.com/linuxfoundation/lfx-v2-mailing-list-service/internal/domain/model" + pkgerrors "github.com/linuxfoundation/lfx-v2-mailing-list-service/pkg/errors" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestErrorSimulation(t *testing.T) { + ctx := context.Background() + + t.Run("Service error simulation", func(t *testing.T) { + repo := NewMockRepository() + + // Configure error for specific service + serviceUID := "test-service-uid" + expectedErr := pkgerrors.NewNotFound("simulated service not found") + repo.SetErrorForService(serviceUID, expectedErr) + + // Try to get the service - should return configured error + _, _, err := repo.GetGrpsIOService(ctx, serviceUID) + require.Error(t, err) + assert.True(t, errors.Is(err, expectedErr)) + }) + + t.Run("Mailing list error simulation", func(t *testing.T) { + repo := NewMockRepository() + + // Configure error for specific mailing list + mailingListUID := "test-mailinglist-uid" + expectedErr := pkgerrors.NewConflict("simulated mailing list conflict") + repo.SetErrorForMailingList(mailingListUID, expectedErr) + + // Try to get the mailing list - should return configured error + _, _, err := repo.GetGrpsIOMailingList(ctx, mailingListUID) + require.Error(t, err) + assert.True(t, errors.Is(err, expectedErr)) + }) + + t.Run("Member error simulation", func(t *testing.T) { + repo := NewMockRepository() + + // Configure error for specific member + memberUID := "test-member-uid" + expectedErr := pkgerrors.NewUnauthorized("simulated member unauthorized") + repo.SetErrorForMember(memberUID, expectedErr) + + // Try to get the member - should return configured error + _, _, err := repo.GetGrpsIOMember(ctx, memberUID) + require.Error(t, err) + assert.True(t, errors.Is(err, expectedErr)) + }) + + t.Run("Operation error simulation", func(t *testing.T) { + repo := NewMockRepository() + + // Configure error for specific operation + expectedErr := pkgerrors.NewServiceUnavailable("simulated service unavailable") + repo.SetErrorForOperation("CreateGrpsIOMailingList", expectedErr) + + // Try to create a mailing list - should return configured error + mailingList := &model.GrpsIOMailingList{ + UID: "test-create-uid", + GroupName: "test-list", + Type: "discussion_open", + } + _, _, err := repo.CreateGrpsIOMailingList(ctx, mailingList) + require.Error(t, err) + assert.True(t, errors.Is(err, expectedErr)) + }) + + t.Run("Global error simulation", func(t *testing.T) { + repo := NewMockRepository() + + // Configure global error for all operations + expectedErr := pkgerrors.NewUnexpected("simulated global error") + repo.SetGlobalError(expectedErr) + + // Try any operation - should return configured global error + _, _, err := repo.GetGrpsIOService(ctx, "any-service") + require.Error(t, err) + assert.True(t, errors.Is(err, expectedErr)) + + // Try another operation - should also return global error + _, _, err = repo.GetGrpsIOMailingList(ctx, "any-mailinglist") + require.Error(t, err) + assert.True(t, errors.Is(err, expectedErr)) + }) + + t.Run("Clear error simulation", func(t *testing.T) { + repo := NewMockRepository() + + // Configure some errors + repo.SetErrorForService("test-service", pkgerrors.NewNotFound("test error")) + repo.SetGlobalError(pkgerrors.NewUnexpected("global error")) + + // Clear all error simulation + repo.ClearErrorSimulation() + + // Operations should work normally now (return NotFound for non-existent resources) + _, _, err := repo.GetGrpsIOService(ctx, "non-existent-service") + require.Error(t, err) + + // Should be NotFound error from normal logic, not our simulated error + var notFoundErr pkgerrors.NotFound + assert.True(t, errors.As(err, ¬FoundErr)) + assert.Contains(t, err.Error(), "service with UID non-existent-service not found") + }) + + t.Run("Error priority - global takes precedence", func(t *testing.T) { + repo := NewMockRepository() + + // Set both specific and global errors + serviceUID := "test-service" + specificErr := pkgerrors.NewNotFound("specific error") + globalErr := pkgerrors.NewUnexpected("global error") + + repo.SetErrorForService(serviceUID, specificErr) + repo.SetGlobalError(globalErr) + + // Global error should take precedence + _, _, err := repo.GetGrpsIOService(ctx, serviceUID) + require.Error(t, err) + assert.True(t, errors.Is(err, globalErr)) + assert.False(t, errors.Is(err, specificErr)) + }) + + t.Run("Error priority - operation over resource", func(t *testing.T) { + repo := NewMockRepository() + + // Clear any existing error simulation first + repo.ClearErrorSimulation() + + // Set both operation and resource-specific errors + serviceUID := "test-service" + resourceErr := pkgerrors.NewNotFound("resource error") + operationErr := pkgerrors.NewConflict("operation error") + + repo.SetErrorForService(serviceUID, resourceErr) + repo.SetErrorForOperation("GetGrpsIOService", operationErr) + + // Operation error should take precedence + _, _, err := repo.GetGrpsIOService(ctx, serviceUID) + require.Error(t, err) + assert.True(t, errors.Is(err, operationErr)) + assert.False(t, errors.Is(err, resourceErr)) + }) +} diff --git a/internal/infrastructure/mock/grpsio.go b/internal/infrastructure/mock/grpsio.go index 867020e..18e5c1c 100644 --- a/internal/infrastructure/mock/grpsio.go +++ b/internal/infrastructure/mock/grpsio.go @@ -7,11 +7,13 @@ import ( "context" "fmt" "log/slog" + "strings" "sync" "time" "github.com/linuxfoundation/lfx-v2-mailing-list-service/internal/domain/model" "github.com/linuxfoundation/lfx-v2-mailing-list-service/internal/domain/port" + "github.com/linuxfoundation/lfx-v2-mailing-list-service/internal/infrastructure/groupsio" "github.com/linuxfoundation/lfx-v2-mailing-list-service/pkg/constants" "github.com/linuxfoundation/lfx-v2-mailing-list-service/pkg/errors" ) @@ -22,6 +24,16 @@ var ( globalMockRepoOnce = &sync.Once{} ) +// ErrorSimulationConfig configures error simulation for testing scenarios +type ErrorSimulationConfig struct { + Enabled bool `json:"enabled"` + ServiceErrors map[string]error `json:"-"` // service_uid -> error to return + MailingListErrors map[string]error `json:"-"` // mailing_list_uid -> error to return + MemberErrors map[string]error `json:"-"` // member_uid -> error to return + GlobalError error `json:"-"` // error for all operations + OperationErrors map[string]error `json:"-"` // operation_name -> error +} + // MockRepository provides a mock implementation of all repository interfaces for testing type MockRepository struct { services map[string]*model.GrpsIOService @@ -36,6 +48,8 @@ type MockRepository struct { projectSlugs map[string]string // projectUID -> slug projectNames map[string]string // projectUID -> name committeeNames map[string]string // committeeUID -> name + errorSimulation ErrorSimulationConfig // Error simulation configuration + errorSimulationMu sync.RWMutex // Protect concurrent access to error config mu sync.RWMutex // Protect concurrent access to maps } @@ -58,6 +72,14 @@ func NewMockRepository() *MockRepository { projectSlugs: make(map[string]string), projectNames: make(map[string]string), committeeNames: make(map[string]string), + errorSimulation: ErrorSimulationConfig{ + Enabled: false, + ServiceErrors: make(map[string]error), + MailingListErrors: make(map[string]error), + MemberErrors: make(map[string]error), + GlobalError: nil, + OperationErrors: make(map[string]error), + }, } // Add sample project data for testing @@ -94,9 +116,9 @@ func NewMockRepository() *MockRepository { sampleServices := []*model.GrpsIOService{ { Type: "primary", - UID: "service-1", + UID: "550e8400-e29b-41d4-a716-446655440001", Domain: "lists.testproject.org", - GroupID: 12345, + GroupID: func() *int64 { id := int64(12345); return &id }(), Status: "created", GlobalOwners: []string{"admin@testproject.org"}, Prefix: "", @@ -110,9 +132,9 @@ func NewMockRepository() *MockRepository { }, { Type: "formation", - UID: "service-2", + UID: "550e8400-e29b-41d4-a716-446655440002", Domain: "lists.formation.testproject.org", - GroupID: 12346, + GroupID: func() *int64 { id := int64(12346); return &id }(), Status: "created", GlobalOwners: []string{"formation@testproject.org"}, Prefix: "formation", @@ -126,9 +148,9 @@ func NewMockRepository() *MockRepository { }, { Type: "primary", - UID: "service-3", + UID: "550e8400-e29b-41d4-a716-446655440003", Domain: "lists.example.org", - GroupID: 12347, + GroupID: func() *int64 { id := int64(12347); return &id }(), Status: "pending", GlobalOwners: []string{"owner@example.org", "admin@example.org"}, Prefix: "", @@ -149,9 +171,9 @@ func NewMockRepository() *MockRepository { mock.serviceIndexKeys[service.BuildIndexKey(context.Background())] = service } - // Add project mappings - mock.projectSlugs["7cad5a8d-19d0-41a4-81a6-043453daf9ee"] = "test-project" - mock.projectNames["7cad5a8d-19d0-41a4-81a6-043453daf9ee"] = "Test Project" + // Add project mappings - using consistent naming + mock.projectSlugs["7cad5a8d-19d0-41a4-81a6-043453daf9ee"] = "sample-project" + mock.projectNames["7cad5a8d-19d0-41a4-81a6-043453daf9ee"] = "Cloud Native Computing Foundation" mock.projectSlugs["8dbc6b9e-20e1-42b5-92b7-154564eaf0ff"] = "example-project" mock.projectNames["8dbc6b9e-20e1-42b5-92b7-154564eaf0ff"] = "Example Project" @@ -168,7 +190,7 @@ func NewMockRepository() *MockRepository { Description: "Development discussions and technical matters for the project", Title: "Development List", SubjectTag: "[DEV]", - ServiceUID: "service-1", + ServiceUID: "550e8400-e29b-41d4-a716-446655440001", ProjectUID: "7cad5a8d-19d0-41a4-81a6-043453daf9ee", Writers: []string{"dev-admin@testproject.org"}, Auditors: []string{"auditor@testproject.org"}, @@ -183,7 +205,7 @@ func NewMockRepository() *MockRepository { Description: "Official announcements and project news for all stakeholders", Title: "Announcements", SubjectTag: "[ANNOUNCE]", - ServiceUID: "service-1", + ServiceUID: "550e8400-e29b-41d4-a716-446655440001", ProjectUID: "7cad5a8d-19d0-41a4-81a6-043453daf9ee", Writers: []string{"admin@testproject.org"}, Auditors: []string{"auditor@testproject.org"}, @@ -201,7 +223,7 @@ func NewMockRepository() *MockRepository { Description: "Private security discussions for committee members only", Title: "Formation Security List", SubjectTag: "[SECURITY]", - ServiceUID: "service-2", + ServiceUID: "550e8400-e29b-41d4-a716-446655440002", ProjectUID: "7cad5a8d-19d0-41a4-81a6-043453daf9ee", Writers: []string{"security@testproject.org"}, Auditors: []string{"security-audit@testproject.org"}, @@ -392,11 +414,11 @@ func (w *MockGrpsIOWriter) UpdateGrpsIOMember(ctx context.Context, uid string, m // Update member while preserving immutable fields memberCopy := *member - memberCopy.UID = existing.UID // Preserve UID - memberCopy.Email = existing.Email // Preserve email (immutable) + memberCopy.UID = existing.UID // Preserve UID + memberCopy.Email = existing.Email // Preserve email (immutable) memberCopy.MailingListUID = existing.MailingListUID // Preserve mailing list UID (immutable) - memberCopy.CreatedAt = existing.CreatedAt // Preserve created timestamp - memberCopy.UpdatedAt = time.Now() // Update timestamp + memberCopy.CreatedAt = existing.CreatedAt // Preserve created timestamp + memberCopy.UpdatedAt = time.Now() // Update timestamp // Store updated member and increment revision w.mock.members[uid] = &memberCopy @@ -494,10 +516,112 @@ func (r *MockEntityAttributeReader) CommitteeName(ctx context.Context, uid strin return name, nil } +// Error configuration methods for testing + +// SetErrorForService configures the mock to return an error for a specific service UID +func (m *MockRepository) SetErrorForService(serviceUID string, err error) { + m.errorSimulationMu.Lock() + defer m.errorSimulationMu.Unlock() + + m.errorSimulation.Enabled = true + m.errorSimulation.ServiceErrors[serviceUID] = err +} + +// SetErrorForMailingList configures the mock to return an error for a specific mailing list UID +func (m *MockRepository) SetErrorForMailingList(mailingListUID string, err error) { + m.errorSimulationMu.Lock() + defer m.errorSimulationMu.Unlock() + + m.errorSimulation.Enabled = true + m.errorSimulation.MailingListErrors[mailingListUID] = err +} + +// SetErrorForMember configures the mock to return an error for a specific member UID +func (m *MockRepository) SetErrorForMember(memberUID string, err error) { + m.errorSimulationMu.Lock() + defer m.errorSimulationMu.Unlock() + + m.errorSimulation.Enabled = true + m.errorSimulation.MemberErrors[memberUID] = err +} + +// SetErrorForOperation configures the mock to return an error for a specific operation +func (m *MockRepository) SetErrorForOperation(operation string, err error) { + m.errorSimulationMu.Lock() + defer m.errorSimulationMu.Unlock() + + m.errorSimulation.Enabled = true + m.errorSimulation.OperationErrors[operation] = err +} + +// SetGlobalError configures the mock to return an error for all operations +func (m *MockRepository) SetGlobalError(err error) { + m.errorSimulationMu.Lock() + defer m.errorSimulationMu.Unlock() + + m.errorSimulation.Enabled = true + m.errorSimulation.GlobalError = err +} + +// ClearErrorSimulation disables all error simulation +func (m *MockRepository) ClearErrorSimulation() { + m.errorSimulationMu.Lock() + defer m.errorSimulationMu.Unlock() + + m.errorSimulation.Enabled = false + m.errorSimulation.ServiceErrors = make(map[string]error) + m.errorSimulation.MailingListErrors = make(map[string]error) + m.errorSimulation.MemberErrors = make(map[string]error) + m.errorSimulation.OperationErrors = make(map[string]error) + m.errorSimulation.GlobalError = nil +} + +// checkErrorSimulation checks if an error should be returned based on configuration +func (m *MockRepository) checkErrorSimulation(operation, resourceUID string) error { + m.errorSimulationMu.RLock() + defer m.errorSimulationMu.RUnlock() + + if !m.errorSimulation.Enabled { + return nil + } + + // Check global error first + if m.errorSimulation.GlobalError != nil { + return m.errorSimulation.GlobalError + } + + // Check operation-specific error + if err, exists := m.errorSimulation.OperationErrors[operation]; exists { + return err + } + + // Check resource-specific errors based on operation type + if strings.Contains(operation, "Service") { + if err, exists := m.errorSimulation.ServiceErrors[resourceUID]; exists { + return err + } + } else if strings.Contains(operation, "MailingList") { + if err, exists := m.errorSimulation.MailingListErrors[resourceUID]; exists { + return err + } + } else if strings.Contains(operation, "Member") { + if err, exists := m.errorSimulation.MemberErrors[resourceUID]; exists { + return err + } + } + + return nil +} + // GetGrpsIOService retrieves a single service by ID and returns ETag revision func (m *MockRepository) GetGrpsIOService(ctx context.Context, uid string) (*model.GrpsIOService, uint64, error) { slog.DebugContext(ctx, "mock service: getting service", "service_uid", uid) + // Check error simulation first + if err := m.checkErrorSimulation("GetGrpsIOService", uid); err != nil { + return nil, 0, err + } + m.mu.RLock() defer m.mu.RUnlock() @@ -828,6 +952,126 @@ func NewMockGrpsIOMessagePublisher() port.MessagePublisher { return &MockGrpsIOMessagePublisher{} } +// MockGroupsIOClient provides a mock implementation of the GroupsIO HTTP client +type MockGroupsIOClient struct { + CallLog []string // Track method calls for testing +} + +// Verify MockGroupsIOClient implements ClientInterface +var _ groupsio.ClientInterface = (*MockGroupsIOClient)(nil) + +// NewMockGroupsIOClient creates a new mock GroupsIO client +func NewMockGroupsIOClient() *MockGroupsIOClient { + return &MockGroupsIOClient{ + CallLog: make([]string, 0), + } +} + +// CreateGroup mocks the Groups.io group creation API +func (m *MockGroupsIOClient) CreateGroup(ctx context.Context, domain string, options groupsio.GroupCreateOptions) (*groupsio.GroupObject, error) { + m.CallLog = append(m.CallLog, fmt.Sprintf("CreateGroup(domain=%s, group_name=%s)", domain, options.GroupName)) + + // Return mock result + return &groupsio.GroupObject{ + ID: 12345, // Mock group ID + }, nil +} + +// UpdateGroup mocks the Groups.io group update API +func (m *MockGroupsIOClient) UpdateGroup(ctx context.Context, domain string, groupID uint64, updates groupsio.GroupUpdateOptions) error { + m.CallLog = append(m.CallLog, fmt.Sprintf("UpdateGroup(domain=%s, group_id=%d, owners=%v)", domain, groupID, updates.GlobalOwners)) + + slog.InfoContext(ctx, "[MOCK] Groups.io group update simulated", + "domain", domain, "group_id", groupID, "global_owners", updates.GlobalOwners) + + return nil +} + +// UpdateMember mocks the Groups.io member update API +func (m *MockGroupsIOClient) UpdateMember(ctx context.Context, domain string, memberID uint64, updates groupsio.MemberUpdateOptions) error { + m.CallLog = append(m.CallLog, fmt.Sprintf("UpdateMember(domain=%s, member_id=%d, mod_status=%s)", domain, memberID, updates.ModStatus)) + + slog.InfoContext(ctx, "[MOCK] Groups.io member update simulated", + "domain", domain, "member_id", memberID, "mod_status", updates.ModStatus, "delivery", updates.DeliveryMode) + + return nil +} + +// UpdateSubgroup mocks the Groups.io subgroup update API +func (m *MockGroupsIOClient) UpdateSubgroup(ctx context.Context, domain string, subgroupID uint64, updates groupsio.SubgroupUpdateOptions) error { + m.CallLog = append(m.CallLog, fmt.Sprintf("UpdateSubgroup(domain=%s, subgroup_id=%d, title=%s)", domain, subgroupID, updates.Title)) + + slog.InfoContext(ctx, "[MOCK] Groups.io subgroup update simulated", + "domain", domain, "subgroup_id", subgroupID, "title", updates.Title, "description", updates.Description) + + return nil +} + +// DeleteGroup mocks the Groups.io group deletion API +func (m *MockGroupsIOClient) DeleteGroup(ctx context.Context, domain string, groupID uint64) error { + m.CallLog = append(m.CallLog, fmt.Sprintf("DeleteGroup(domain=%s, group_id=%d)", domain, groupID)) + + slog.InfoContext(ctx, "[MOCK] Groups.io group deletion simulated", + "domain", domain, "group_id", groupID) + + return nil +} + +// CreateSubgroup mocks the Groups.io subgroup creation API +func (m *MockGroupsIOClient) CreateSubgroup(ctx context.Context, domain string, parentGroupID uint64, options groupsio.SubgroupCreateOptions) (*groupsio.SubgroupObject, error) { + m.CallLog = append(m.CallLog, fmt.Sprintf("CreateSubgroup(domain=%s, parent_id=%d, name=%s)", domain, parentGroupID, options.GroupName)) + + // Return mock result + return &groupsio.SubgroupObject{ + ID: 67890, // Mock subgroup ID + }, nil +} + +// DeleteSubgroup mocks the Groups.io subgroup deletion API +func (m *MockGroupsIOClient) DeleteSubgroup(ctx context.Context, domain string, subgroupID uint64) error { + m.CallLog = append(m.CallLog, fmt.Sprintf("DeleteSubgroup(domain=%s, subgroup_id=%d)", domain, subgroupID)) + + slog.InfoContext(ctx, "[MOCK] Groups.io subgroup deletion simulated", + "domain", domain, "subgroup_id", subgroupID) + + return nil +} + +// AddMember mocks the Groups.io member addition API +func (m *MockGroupsIOClient) AddMember(ctx context.Context, domain string, subgroupID uint64, email, name string) (*groupsio.MemberObject, error) { + m.CallLog = append(m.CallLog, fmt.Sprintf("AddMember(domain=%s, subgroup_id=%d, email=%s)", domain, subgroupID, email)) + + // Return mock result + return &groupsio.MemberObject{ + ID: 11111, // Mock member ID + }, nil +} + +// RemoveMember mocks the Groups.io member removal API +func (m *MockGroupsIOClient) RemoveMember(ctx context.Context, domain string, memberID uint64) error { + m.CallLog = append(m.CallLog, fmt.Sprintf("RemoveMember(domain=%s, member_id=%d)", domain, memberID)) + + slog.InfoContext(ctx, "[MOCK] Groups.io member removal simulated", + "domain", domain, "member_id", memberID) + + return nil +} + +// IsReady mocks the readiness check +func (m *MockGroupsIOClient) IsReady(ctx context.Context) error { + return nil // Mock client is always ready +} + +// GetCallLog returns the log of method calls for testing +func (m *MockGroupsIOClient) GetCallLog() []string { + return m.CallLog +} + +// ClearCallLog clears the call log +func (m *MockGroupsIOClient) ClearCallLog() { + m.CallLog = make([]string, 0) +} + // GetServiceCount returns the total number of services func (m *MockRepository) GetServiceCount() int { m.mu.RLock() @@ -852,6 +1096,11 @@ func (m *MockRepository) GetMailingListRevision(ctx context.Context, uid string) func (m *MockRepository) GetGrpsIOMailingListWithRevision(ctx context.Context, uid string) (*model.GrpsIOMailingList, uint64, error) { slog.DebugContext(ctx, "mock mailing list: getting mailing list with revision", "mailing_list_uid", uid) + // Check error simulation first + if err := m.checkErrorSimulation("GetGrpsIOMailingList", uid); err != nil { + return nil, 0, err + } + m.mu.RLock() defer m.mu.RUnlock() @@ -916,6 +1165,11 @@ func (m *MockRepository) CheckMailingListExists(ctx context.Context, parentID, g func (m *MockRepository) CreateGrpsIOMailingList(ctx context.Context, mailingList *model.GrpsIOMailingList) (*model.GrpsIOMailingList, uint64, error) { slog.DebugContext(ctx, "mock mailing list: creating mailing list", "mailing_list_id", mailingList.UID) + // Check error simulation first + if err := m.checkErrorSimulation("CreateGrpsIOMailingList", mailingList.UID); err != nil { + return nil, 0, err + } + m.mu.Lock() defer m.mu.Unlock() @@ -954,6 +1208,11 @@ func (m *MockRepository) CreateGrpsIOMailingList(ctx context.Context, mailingLis func (m *MockRepository) UpdateGrpsIOMailingList(ctx context.Context, mailingList *model.GrpsIOMailingList) (*model.GrpsIOMailingList, error) { slog.DebugContext(ctx, "mock mailing list: updating mailing list", "mailing_list_uid", mailingList.UID) + // Check error simulation first + if err := m.checkErrorSimulation("UpdateGrpsIOMailingList", mailingList.UID); err != nil { + return nil, err + } + m.mu.Lock() defer m.mu.Unlock() @@ -1047,6 +1306,11 @@ func (m *MockRepository) UpdateGrpsIOMailingListWithRevision(ctx context.Context func (m *MockRepository) DeleteGrpsIOMailingList(ctx context.Context, uid string) error { slog.DebugContext(ctx, "mock mailing list: deleting mailing list", "mailing_list_uid", uid) + // Check error simulation first + if err := m.checkErrorSimulation("DeleteGrpsIOMailingList", uid); err != nil { + return err + } + m.mu.Lock() defer m.mu.Unlock() @@ -1158,6 +1422,11 @@ func (m *MockRepository) GetMailingListCount() int { func (m *MockRepository) GetGrpsIOMember(ctx context.Context, uid string) (*model.GrpsIOMember, uint64, error) { slog.DebugContext(ctx, "mock member: getting member", "member_uid", uid) + // Check error simulation first + if err := m.checkErrorSimulation("GetGrpsIOMember", uid); err != nil { + return nil, 0, err + } + m.mu.RLock() defer m.mu.RUnlock() @@ -1186,7 +1455,6 @@ func (m *MockRepository) GetMemberRevision(ctx context.Context, uid string) (uin return 0, errors.NewNotFound("member not found") } - // AddMember adds a member to the mock repository for testing func (m *MockRepository) AddMember(member *model.GrpsIOMember) { m.mu.Lock() diff --git a/internal/service/grpsio_mailing_list_writer.go b/internal/service/grpsio_mailing_list_writer.go index 806c501..f454246 100644 --- a/internal/service/grpsio_mailing_list_writer.go +++ b/internal/service/grpsio_mailing_list_writer.go @@ -11,8 +11,10 @@ import ( "github.com/google/uuid" "github.com/linuxfoundation/lfx-v2-mailing-list-service/internal/domain/model" + "github.com/linuxfoundation/lfx-v2-mailing-list-service/internal/infrastructure/groupsio" "github.com/linuxfoundation/lfx-v2-mailing-list-service/pkg/constants" "github.com/linuxfoundation/lfx-v2-mailing-list-service/pkg/errors" + "github.com/linuxfoundation/lfx-v2-mailing-list-service/pkg/utils" ) // CreateGrpsIOMailingList creates a new mailing list with comprehensive validation and messaging @@ -24,12 +26,21 @@ func (ml *grpsIOWriterOrchestrator) CreateGrpsIOMailingList(ctx context.Context, // For rollback purposes var ( - keys []string - rollbackRequired bool + keys []string + rollbackRequired bool + rollbackSubgroupID *int64 + rollbackGroupsIODomain string ) defer func() { if err := recover(); err != nil || rollbackRequired { ml.deleteKeys(ctx, keys, true) + + // Clean up GroupsIO subgroup if created + if rollbackSubgroupID != nil && ml.groupsClient != nil { + if deleteErr := ml.groupsClient.DeleteSubgroup(ctx, rollbackGroupsIODomain, utils.Int64PtrToUint64(rollbackSubgroupID)); deleteErr != nil { + slog.WarnContext(ctx, "failed to cleanup GroupsIO subgroup during rollback", "error", deleteErr, "subgroup_id", *rollbackSubgroupID) + } + } } }() @@ -60,24 +71,24 @@ func (ml *grpsIOWriterOrchestrator) CreateGrpsIOMailingList(ctx context.Context, return nil, 0, err } - // Step 4: Validate parent service and inherit metadata + // Step 5: Validate parent service and inherit metadata parentService, err := ml.validateAndInheritFromParent(ctx, request) if err != nil { return nil, 0, err } - // Step 5: Validate committee and populate metadata (if specified) + // Step 6: Validate committee and populate metadata (if specified) if err := ml.validateAndPopulateCommittee(ctx, request, parentService.ProjectUID); err != nil { return nil, 0, err } - // Step 6: Validate group name prefix for non-primary services + // Step 7: Validate group name prefix for non-primary services if err := request.ValidateGroupNamePrefix(parentService.Type, parentService.Prefix); err != nil { slog.WarnContext(ctx, "group name prefix validation failed", "error", err) return nil, 0, err } - // Step 7: Reserve unique constraints (like service pattern) + // Step 8: Reserve unique constraints constraintKey, err := ml.reserveMailingListConstraints(ctx, request) if err != nil { rollbackRequired = true @@ -87,7 +98,27 @@ func (ml *grpsIOWriterOrchestrator) CreateGrpsIOMailingList(ctx context.Context, keys = append(keys, constraintKey) } - // Step 8: Create mailing list in storage + // Step 9: Create in Groups.io FIRST (if enabled) + subgroupID, err := ml.createMailingListInGroupsIO(ctx, request, parentService) + if err != nil { + rollbackRequired = true + return nil, 0, err + } + + if subgroupID != nil { + // Groups.io creation successful - track for rollback cleanup + rollbackSubgroupID = subgroupID + rollbackGroupsIODomain = parentService.Domain + + request.SubgroupID = subgroupID + request.SyncStatus = "synced" + } else { + // Mock/disabled mode or parent service has no GroupID - set appropriate status + request.SyncStatus = "pending" + slog.InfoContext(ctx, "Groups.io integration disabled or parent service not synced - mailing list will be in pending state") + } + + // Step 10: Create mailing list in storage (with Groups.io ID already set) createdMailingList, revision, err := ml.grpsIOWriter.CreateGrpsIOMailingList(ctx, request) if err != nil { slog.ErrorContext(ctx, "failed to create mailing list in storage", "error", err) @@ -96,17 +127,18 @@ func (ml *grpsIOWriterOrchestrator) CreateGrpsIOMailingList(ctx context.Context, } keys = append(keys, createdMailingList.UID) - // Step 8.1: Create secondary indices for the mailing list + // Step 11: Create secondary indices for the mailing list secondaryKeys, err := ml.createMailingListSecondaryIndices(ctx, createdMailingList) if err != nil { slog.ErrorContext(ctx, "failed to create mailing list secondary indices", "error", err) rollbackRequired = true return nil, 0, err } + // Add secondary keys to rollback list keys = append(keys, secondaryKeys...) - // Step 9: Publish messages concurrently (indexer + access control) + // Step 12: Publish messages concurrently (indexer + access control) if err := ml.publishMailingListMessages(ctx, createdMailingList); err != nil { // Log warning but don't fail the operation - mailing list is already created slog.WarnContext(ctx, "failed to publish messages", "error", err, "mailing_list_uid", createdMailingList.UID) @@ -122,6 +154,51 @@ func (ml *grpsIOWriterOrchestrator) CreateGrpsIOMailingList(ctx context.Context, return createdMailingList, revision, nil } +// createMailingListInGroupsIO handles Groups.io subgroup creation and returns the ID +func (ml *grpsIOWriterOrchestrator) createMailingListInGroupsIO(ctx context.Context, mailingList *model.GrpsIOMailingList, parentService *model.GrpsIOService) (*int64, error) { + if ml.groupsClient == nil || parentService.GroupID == nil { + return nil, nil // Skip Groups.io creation + } + + slog.InfoContext(ctx, "creating subgroup in Groups.io", + "domain", parentService.Domain, + "parent_group_id", *parentService.GroupID, + "subgroup_name", mailingList.GroupName, + ) + + subgroupOptions := groupsio.SubgroupCreateOptions{ + ParentGroupID: utils.Int64PtrToUint64(parentService.GroupID), // Production field + GroupName: mailingList.GroupName, // Fixed: was SubgroupName + Desc: fmt.Sprintf("Mailing list for %s - %s", parentService.ProjectName, mailingList.GroupName), // Fixed: was Description + // Privacy: leave empty to inherit from parent group (production pattern) + } + + subgroupResult, err := ml.groupsClient.CreateSubgroup( + ctx, + parentService.Domain, + utils.Int64PtrToUint64(parentService.GroupID), + subgroupOptions, + ) + if err != nil { + slog.ErrorContext(ctx, "Groups.io subgroup creation failed", + "error", err, + "domain", parentService.Domain, + "parent_group_id", *parentService.GroupID, + "subgroup_name", mailingList.GroupName, + ) + return nil, fmt.Errorf("groups.io subgroup creation failed: %w", err) + } + + subgroupID := int64(subgroupResult.ID) + slog.InfoContext(ctx, "Groups.io subgroup created successfully", + "subgroup_id", subgroupResult.ID, + "domain", parentService.Domain, + "parent_group_id", *parentService.GroupID, + ) + + return &subgroupID, nil +} + // validateAndInheritFromParent validates parent service exists and inherits metadata func (ml *grpsIOWriterOrchestrator) validateAndInheritFromParent(ctx context.Context, request *model.GrpsIOMailingList) (*model.GrpsIOService, error) { slog.DebugContext(ctx, "validating parent service", "parent_uid", request.ServiceUID) @@ -328,6 +405,9 @@ func (ml *grpsIOWriterOrchestrator) UpdateGrpsIOMailingList(ctx context.Context, "revision", newRevision, ) + // Sync mailing list updates to Groups.io + ml.syncMailingListToGroupsIO(ctx, updatedMailingList) + // Publish update messages if ml.publisher != nil { if err := ml.publishMailingListUpdateMessages(ctx, updatedMailingList); err != nil { @@ -454,6 +534,9 @@ func (ml *grpsIOWriterOrchestrator) DeleteGrpsIOMailingList(ctx context.Context, "group_name", mailingListData.GroupName, "public", mailingListData.Public) + // Step 2.1: Delete subgroup from Groups.io (if client available and mailing list has SubgroupID) + ml.deleteSubgroupWithCleanup(ctx, mailingListData.ServiceUID, mailingListData.SubgroupID) + // Delete from storage with revision check err := ml.grpsIOWriter.DeleteGrpsIOMailingList(ctx, uid, expectedRevision, mailingListData) if err != nil { @@ -502,3 +585,38 @@ func (ml *grpsIOWriterOrchestrator) mergeMailingListData(ctx context.Context, ex "mutable_fields", []string{"public", "type", "description", "title", "committee_uid", "committee_name", "committee_filters", "subject_tag", "writers", "auditors", "last_reviewed_at", "last_reviewed_by"}, ) } + +// syncMailingListToGroupsIO handles Groups.io mailing list update with proper error handling +func (ml *grpsIOWriterOrchestrator) syncMailingListToGroupsIO(ctx context.Context, mailingList *model.GrpsIOMailingList) { + // Guard clause: skip if Groups.io client not available or mailing list not synced + if ml.groupsClient == nil || mailingList.SubgroupID == nil { + slog.InfoContext(ctx, "Groups.io integration disabled or mailing list not synced - skipping Groups.io update") + return + } + + // Get domain using helper method + domain, err := ml.getGroupsIODomainForResource(ctx, mailingList.UID, constants.ResourceTypeMailingList) + if err != nil { + slog.WarnContext(ctx, "Groups.io mailing list sync skipped due to domain lookup failure, local update will proceed", + "error", err, "mailing_list_uid", mailingList.UID) + return + } + + // Build update options from mailing list model + updates := groupsio.SubgroupUpdateOptions{ + Title: mailingList.Title, + Description: mailingList.Description, + SubjectTag: mailingList.SubjectTag, + Committee: mailingList.CommitteeUID, + } + + // Perform Groups.io mailing list update + err = ml.groupsClient.UpdateSubgroup(ctx, domain, utils.Int64PtrToUint64(mailingList.SubgroupID), updates) + if err != nil { + slog.WarnContext(ctx, "Groups.io mailing list update failed, local update will proceed", + "error", err, "domain", domain, "subgroup_id", *mailingList.SubgroupID) + } else { + slog.InfoContext(ctx, "Groups.io mailing list updated successfully", + "subgroup_id", *mailingList.SubgroupID, "domain", domain) + } +} diff --git a/internal/service/grpsio_mailing_list_writer_test.go b/internal/service/grpsio_mailing_list_writer_test.go index 41bc1ab..9c67208 100644 --- a/internal/service/grpsio_mailing_list_writer_test.go +++ b/internal/service/grpsio_mailing_list_writer_test.go @@ -944,4 +944,87 @@ func TestGrpsIOWriterOrchestrator_mergeMailingListData(t *testing.T) { } } +// TestGrpsIOWriterOrchestrator_syncMailingListToGroupsIO tests the syncMailingListToGroupsIO method +func TestGrpsIOWriterOrchestrator_syncMailingListToGroupsIO(t *testing.T) { + testCases := []struct { + name string + setupMocks func() (*grpsIOWriterOrchestrator, *mock.MockRepository) + mailingList *model.GrpsIOMailingList + expectSkip bool + expectWarning bool + validateLogs func(t *testing.T) + }{ + { + name: "skip sync when Groups.io client is nil", + setupMocks: func() (*grpsIOWriterOrchestrator, *mock.MockRepository) { + mockRepo := mock.NewMockRepository() + orchestrator := &grpsIOWriterOrchestrator{ + groupsClient: nil, // No client + } + return orchestrator, mockRepo + }, + mailingList: &model.GrpsIOMailingList{ + UID: "mailing-list-1", + SubgroupID: func() *int64 { i := int64(12345); return &i }(), + Title: "Test Mailing List", + }, + expectSkip: true, + }, + { + name: "skip sync when mailing list SubgroupID is nil", + setupMocks: func() (*grpsIOWriterOrchestrator, *mock.MockRepository) { + mockRepo := mock.NewMockRepository() + orchestrator := &grpsIOWriterOrchestrator{ + groupsClient: nil, // Could be any client, but SubgroupID is nil + } + return orchestrator, mockRepo + }, + mailingList: &model.GrpsIOMailingList{ + UID: "mailing-list-2", + SubgroupID: nil, // No subgroup ID - not synced + Title: "Test Mailing List", + }, + expectSkip: true, + }, + { + name: "skip sync when domain lookup fails", + setupMocks: func() (*grpsIOWriterOrchestrator, *mock.MockRepository) { + mockRepo := mock.NewMockRepository() + mockReader := mock.NewMockGrpsIOReader(mockRepo) + + orchestrator := &grpsIOWriterOrchestrator{ + groupsClient: nil, // Any non-nil would work for this test + grpsIOReader: mockReader, + } + + // Note: We don't need actual client for this test as domain lookup will fail first + // The orchestrator has a nil client but that's OK since domain lookup happens first + return orchestrator, mockRepo + }, + mailingList: &model.GrpsIOMailingList{ + UID: "nonexistent-mailing-list", + SubgroupID: func() *int64 { i := int64(12345); return &i }(), + Title: "Test Mailing List", + }, + expectWarning: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + // Setup + ctx := context.Background() + orchestrator, _ := tc.setupMocks() + + // Execute - should not panic regardless of nil clients or missing data + require.NotPanics(t, func() { + orchestrator.syncMailingListToGroupsIO(ctx, tc.mailingList) + }, "syncMailingListToGroupsIO should handle nil clients and missing data gracefully") + + // The method should complete without errors, handling all edge cases internally + // This validates that our error handling and guard clauses work correctly + }) + } +} + // Helper functions (if needed in future) diff --git a/internal/service/grpsio_member_reader.go b/internal/service/grpsio_member_reader.go index 4c50866..bb87ef9 100644 --- a/internal/service/grpsio_member_reader.go +++ b/internal/service/grpsio_member_reader.go @@ -55,4 +55,3 @@ func (r *grpsIOReaderOrchestrator) GetMemberRevision(ctx context.Context, uid st slog.DebugContext(ctx, "member revision retrieved successfully", "member_uid", uid, "revision", revision) return revision, nil } - diff --git a/internal/service/grpsio_member_reader_test.go b/internal/service/grpsio_member_reader_test.go index fe94212..8c57756 100644 --- a/internal/service/grpsio_member_reader_test.go +++ b/internal/service/grpsio_member_reader_test.go @@ -27,8 +27,8 @@ func TestGrpsIOMemberReaderOrchestrator_GetGrpsIOMember(t *testing.T) { testMember := &model.GrpsIOMember{ UID: testMemberUID, MailingListUID: testMailingListUID, - GroupsIOMemberID: 12345, - GroupsIOGroupID: 67890, + GroupsIOMemberID: memberInt64Ptr(12345), + GroupsIOGroupID: memberInt64Ptr(67890), Username: "testuser", FirstName: "John", LastName: "Doe", @@ -66,8 +66,8 @@ func TestGrpsIOMemberReaderOrchestrator_GetGrpsIOMember(t *testing.T) { assert.NotNil(t, member) assert.Equal(t, testMemberUID, member.UID) assert.Equal(t, testMailingListUID, member.MailingListUID) - assert.Equal(t, int64(12345), member.GroupsIOMemberID) - assert.Equal(t, int64(67890), member.GroupsIOGroupID) + assert.Equal(t, memberInt64Ptr(12345), member.GroupsIOMemberID) + assert.Equal(t, memberInt64Ptr(67890), member.GroupsIOGroupID) assert.Equal(t, "testuser", member.Username) assert.Equal(t, "John", member.FirstName) assert.Equal(t, "Doe", member.LastName) @@ -144,8 +144,12 @@ func TestGrpsIOMemberReaderOrchestrator_GetGrpsIOMember(t *testing.T) { } } - // Helper function to create string pointer func memberStringPtr(s string) *string { return &s } + +// Helper function to create int64 pointer +func memberInt64Ptr(i int64) *int64 { + return &i +} diff --git a/internal/service/grpsio_member_writer.go b/internal/service/grpsio_member_writer.go index 1f6c731..93fd62e 100644 --- a/internal/service/grpsio_member_writer.go +++ b/internal/service/grpsio_member_writer.go @@ -11,10 +11,12 @@ import ( "github.com/google/uuid" "github.com/linuxfoundation/lfx-v2-mailing-list-service/internal/domain/model" + "github.com/linuxfoundation/lfx-v2-mailing-list-service/internal/infrastructure/groupsio" "github.com/linuxfoundation/lfx-v2-mailing-list-service/pkg/concurrent" "github.com/linuxfoundation/lfx-v2-mailing-list-service/pkg/constants" errs "github.com/linuxfoundation/lfx-v2-mailing-list-service/pkg/errors" "github.com/linuxfoundation/lfx-v2-mailing-list-service/pkg/redaction" + "github.com/linuxfoundation/lfx-v2-mailing-list-service/pkg/utils" ) // CreateGrpsIOMember creates a new member with transactional operations and rollback following service pattern @@ -41,12 +43,21 @@ func (o *grpsIOWriterOrchestrator) CreateGrpsIOMember(ctx context.Context, membe // For rollback purposes var ( - keys []string - rollbackRequired bool + keys []string + rollbackRequired bool + rollbackMemberID *uint64 + rollbackGroupsIODomain string ) defer func() { if err := recover(); err != nil || rollbackRequired { o.deleteKeys(ctx, keys, true) + + // Clean up GroupsIO member if created + if rollbackMemberID != nil && o.groupsClient != nil { + if deleteErr := o.groupsClient.RemoveMember(ctx, rollbackGroupsIODomain, *rollbackMemberID); deleteErr != nil { + slog.WarnContext(ctx, "failed to cleanup GroupsIO member during rollback", "error", deleteErr, "member_id", *rollbackMemberID) + } + } } }() @@ -63,12 +74,12 @@ func (o *grpsIOWriterOrchestrator) CreateGrpsIOMember(ctx context.Context, membe "mailing_list_uid", member.MailingListUID, ) - // Step 3: Set default status if not provided + // Step 4: Set default status if not provided if member.Status == "" { member.Status = "pending" } - // Step 4: Reserve unique constraints (member email per mailing list) + // Step 5: Reserve unique constraints (member email per mailing list) constraintKey, err := o.grpsIOWriter.UniqueMember(ctx, member) if err != nil { rollbackRequired = true @@ -78,7 +89,46 @@ func (o *grpsIOWriterOrchestrator) CreateGrpsIOMember(ctx context.Context, membe keys = append(keys, constraintKey) } - // Step 5: Create member in storage + // Step 6: Create in Groups.io FIRST (if enabled) + mailingList, _, err := o.grpsIOReader.GetGrpsIOMailingList(ctx, member.MailingListUID) + if err != nil { + slog.ErrorContext(ctx, "failed to get mailing list for Groups.io sync", "error", err, "mailing_list_uid", member.MailingListUID) + rollbackRequired = true + return nil, 0, err + } + + if o.groupsClient != nil && mailingList.SubgroupID != nil { + // Get parent service domain + parentService, _, err := o.grpsIOReader.GetGrpsIOService(ctx, mailingList.ServiceUID) + if err != nil { + slog.ErrorContext(ctx, "failed to get parent service for Groups.io sync", "error", err, "service_uid", mailingList.ServiceUID) + rollbackRequired = true + return nil, 0, err + } + + memberID, groupID, err := o.createMemberInGroupsIO(ctx, member, mailingList, parentService) + if err != nil { + rollbackRequired = true + return nil, 0, err + } + + // Groups.io creation successful - track for rollback cleanup + if memberID != nil { + rollbackMemberID = utils.Int64PtrToUint64Ptr(memberID) + rollbackGroupsIODomain = parentService.Domain + } + + // Set Groups.io IDs on member before storage creation + member.GroupsIOMemberID = memberID + member.GroupsIOGroupID = groupID + member.SyncStatus = "synced" + } else { + // Mock/disabled mode or mailing list not synced - set appropriate status + member.SyncStatus = "pending" + slog.InfoContext(ctx, "Groups.io integration disabled or mailing list not synced - member will be in pending state") + } + + // Step 7: Create member in storage (with Groups.io IDs already set) createdMember, revision, err := o.grpsIOWriter.CreateGrpsIOMember(ctx, member) if err != nil { slog.ErrorContext(ctx, "failed to create member", @@ -96,17 +146,56 @@ func (o *grpsIOWriterOrchestrator) CreateGrpsIOMember(ctx context.Context, membe "revision", revision, ) - // Step 6: Publish messages (indexer and access control) + // Step 8: Publish messages (indexer and access control) if o.publisher != nil { if err := o.publishMemberMessages(ctx, createdMember, model.ActionCreated); err != nil { slog.ErrorContext(ctx, "failed to publish member messages", "error", err) - // Don't rollback on message failure, member creation succeeded + // Don't fail the operation on message failure, member creation succeeded } } return createdMember, revision, nil } +// createMemberInGroupsIO handles Groups.io member creation and returns the IDs +func (o *grpsIOWriterOrchestrator) createMemberInGroupsIO(ctx context.Context, member *model.GrpsIOMember, mailingList *model.GrpsIOMailingList, parentService *model.GrpsIOService) (*int64, *int64, error) { + if o.groupsClient == nil || mailingList.SubgroupID == nil { + return nil, nil, nil // Skip Groups.io creation + } + + slog.InfoContext(ctx, "creating member in Groups.io", + "domain", parentService.Domain, + "subgroup_id", *mailingList.SubgroupID, + "email", member.Email, + ) + + memberResult, err := o.groupsClient.AddMember( + ctx, + parentService.Domain, + utils.Int64PtrToUint64(mailingList.SubgroupID), + member.Email, + fmt.Sprintf("%s %s", member.FirstName, member.LastName), + ) + if err != nil { + slog.ErrorContext(ctx, "Groups.io member creation failed", + "error", err, + "domain", parentService.Domain, + "subgroup_id", *mailingList.SubgroupID, + "email", member.Email, + ) + return nil, nil, fmt.Errorf("groups.io member creation failed: %w", err) + } + + memberID := int64(memberResult.ID) + slog.InfoContext(ctx, "Groups.io member created successfully", + "member_id", memberResult.ID, + "domain", parentService.Domain, + "subgroup_id", *mailingList.SubgroupID, + ) + + return &memberID, mailingList.SubgroupID, nil +} + // UpdateGrpsIOMember updates an existing member following the service pattern with pre-fetch and validation func (o *grpsIOWriterOrchestrator) UpdateGrpsIOMember(ctx context.Context, uid string, member *model.GrpsIOMember, expectedRevision uint64) (*model.GrpsIOMember, uint64, error) { slog.DebugContext(ctx, "executing update member use case", @@ -170,6 +259,15 @@ func (o *grpsIOWriterOrchestrator) UpdateGrpsIOMember(ctx context.Context, uid s "revision", revision, ) + // Step 6.1: Sync member updates to Groups.io (if client available and member has GroupsIOMemberID) + memberUpdates := groupsio.MemberUpdateOptions{ + FirstName: updatedMember.FirstName, + LastName: updatedMember.LastName, + // Note: Email cannot be updated in Groups.io API + // ModStatus and other settings can be added here as needed + } + o.syncMemberToGroupsIO(ctx, updatedMember, memberUpdates) + // Step 7: Publish messages (indexer and access control) if o.publisher != nil { if err := o.publishMemberMessages(ctx, updatedMember, model.ActionUpdated); err != nil { @@ -198,6 +296,9 @@ func (o *grpsIOWriterOrchestrator) DeleteGrpsIOMember(ctx context.Context, uid s slog.DebugContext(ctx, "no member data provided for deletion - will rely on storage layer for validation") } + // Step 1: Remove member from Groups.io (if client available and member has GroupsIOMemberID) + o.removeMemberFromGroupsIO(ctx, member) + // Delete member from storage with optimistic concurrency control err := o.grpsIOWriter.DeleteGrpsIOMember(ctx, uid, expectedRevision, member) if err != nil { @@ -360,3 +461,30 @@ func (o *grpsIOWriterOrchestrator) mergeMemberData(ctx context.Context, existing "mutable_fields", []string{"status", "display_name"}, ) } + +// syncMemberToGroupsIO handles Groups.io member update synchronization with proper error handling +func (o *grpsIOWriterOrchestrator) syncMemberToGroupsIO(ctx context.Context, member *model.GrpsIOMember, updates groupsio.MemberUpdateOptions) { + // Guard clause: skip if Groups.io client not available or member not synced + if o.groupsClient == nil || member.GroupsIOMemberID == nil { + slog.InfoContext(ctx, "Groups.io integration disabled or member not synced - skipping Groups.io update") + return + } + + // Get domain using helper method through member lookup chain + domain, err := o.getGroupsIODomainForResource(ctx, member.UID, constants.ResourceTypeMember) + if err != nil { + slog.WarnContext(ctx, "Groups.io member sync skipped due to domain lookup failure, local update will proceed", + "error", err, "member_uid", member.UID) + return + } + + // Perform Groups.io member update + err = o.groupsClient.UpdateMember(ctx, domain, utils.Int64PtrToUint64(member.GroupsIOMemberID), updates) + if err != nil { + slog.WarnContext(ctx, "Groups.io member update failed, local update will proceed", + "error", err, "domain", domain, "member_id", *member.GroupsIOMemberID) + } else { + slog.InfoContext(ctx, "Groups.io member updated successfully", + "member_id", *member.GroupsIOMemberID, "domain", domain) + } +} diff --git a/internal/service/grpsio_member_writer_test.go b/internal/service/grpsio_member_writer_test.go index 9b93b86..8a78a93 100644 --- a/internal/service/grpsio_member_writer_test.go +++ b/internal/service/grpsio_member_writer_test.go @@ -12,6 +12,7 @@ import ( "github.com/stretchr/testify/require" "github.com/linuxfoundation/lfx-v2-mailing-list-service/internal/domain/model" + "github.com/linuxfoundation/lfx-v2-mailing-list-service/internal/infrastructure/groupsio" "github.com/linuxfoundation/lfx-v2-mailing-list-service/internal/infrastructure/mock" errs "github.com/linuxfoundation/lfx-v2-mailing-list-service/pkg/errors" ) @@ -42,8 +43,8 @@ func TestGrpsIOWriterOrchestrator_CreateGrpsIOMember(t *testing.T) { }, inputMember: &model.GrpsIOMember{ MailingListUID: "mailing-list-1", - GroupsIOMemberID: 12345, - GroupsIOGroupID: 67890, + GroupsIOMemberID: writerInt64Ptr(12345), + GroupsIOGroupID: writerInt64Ptr(67890), Username: "committee-member", FirstName: "Committee", LastName: "Member", @@ -59,7 +60,8 @@ func TestGrpsIOWriterOrchestrator_CreateGrpsIOMember(t *testing.T) { validate: func(t *testing.T, result *model.GrpsIOMember, revision uint64, mockRepo *mock.MockRepository) { assert.NotEmpty(t, result.UID) assert.Equal(t, "mailing-list-1", result.MailingListUID) - assert.Equal(t, int64(12345), result.GroupsIOMemberID) + require.NotNil(t, result.GroupsIOMemberID) + assert.Equal(t, int64(12345), *result.GroupsIOMemberID) assert.Equal(t, "committee-member", result.Username) assert.Equal(t, "Committee", result.FirstName) assert.Equal(t, "Member", result.LastName) @@ -136,7 +138,7 @@ func TestGrpsIOWriterOrchestrator_CreateGrpsIOMember(t *testing.T) { expectedError: nil, validate: func(t *testing.T, result *model.GrpsIOMember, revision uint64, mockRepo *mock.MockRepository) { assert.NotEqual(t, "client-provided-uid", result.UID) // Should NOT preserve client UID - assert.NotEmpty(t, result.UID) // Should have server-generated UID + assert.NotEmpty(t, result.UID) // Should have server-generated UID assert.Equal(t, "mailing-list-3", result.MailingListUID) assert.Equal(t, "normal", result.Status) // Should preserve provided status assert.Equal(t, uint64(1), revision) @@ -200,8 +202,8 @@ func TestGrpsIOWriterOrchestrator_CreateGrpsIOMember(t *testing.T) { }, inputMember: &model.GrpsIOMember{ MailingListUID: "mailing-list-audit", - GroupsIOMemberID: 99999, - GroupsIOGroupID: 88888, + GroupsIOMemberID: writerInt64Ptr(99999), + GroupsIOGroupID: writerInt64Ptr(88888), Username: "audit-member", FirstName: "Audit", LastName: "Member", @@ -219,8 +221,10 @@ func TestGrpsIOWriterOrchestrator_CreateGrpsIOMember(t *testing.T) { validate: func(t *testing.T, result *model.GrpsIOMember, revision uint64, mockRepo *mock.MockRepository) { assert.NotEmpty(t, result.UID) assert.Equal(t, "mailing-list-audit", result.MailingListUID) - assert.Equal(t, int64(99999), result.GroupsIOMemberID) - assert.Equal(t, int64(88888), result.GroupsIOGroupID) + require.NotNil(t, result.GroupsIOMemberID) + assert.Equal(t, int64(99999), *result.GroupsIOMemberID) + require.NotNil(t, result.GroupsIOGroupID) + assert.Equal(t, int64(88888), *result.GroupsIOGroupID) assert.Equal(t, "audit-member", result.Username) assert.Equal(t, "Audit", result.FirstName) assert.Equal(t, "Member", result.LastName) @@ -309,7 +313,7 @@ func TestGrpsIOWriterOrchestrator_UpdateGrpsIOMember(t *testing.T) { MemberType: "committee", Status: "normal", CreatedAt: existingMember.CreatedAt, // Preserve created time - UpdatedAt: time.Now(), // This will be set by the orchestrator + UpdatedAt: time.Now(), // This will be set by the orchestrator } result, revision, err := writer.UpdateGrpsIOMember(ctx, "test-member", updatedMember, 1) @@ -677,3 +681,74 @@ func TestGrpsIOWriterOrchestrator_CreateGrpsIOMember_MemberTypes(t *testing.T) { func writerStringPtr(s string) *string { return &s } + +// Helper function to create int64 pointer +func writerInt64Ptr(i int64) *int64 { + return &i +} + +// TestGrpsIOWriterOrchestrator_syncMemberToGroupsIO tests the syncMemberToGroupsIO method +// Note: This method returns void and only logs errors/warnings. Comprehensive testing would +// require log capture or refactoring the method to return an error/status. +// These tests verify that guard clauses prevent panics in edge cases. +func TestGrpsIOWriterOrchestrator_syncMemberToGroupsIO(t *testing.T) { + testCases := []struct { + name string + setupMocks func() *grpsIOWriterOrchestrator + member *model.GrpsIOMember + updates groupsio.MemberUpdateOptions + }{ + { + name: "skip sync when Groups.io client is nil", + setupMocks: func() *grpsIOWriterOrchestrator { + return &grpsIOWriterOrchestrator{ + groupsClient: nil, // No client - should skip gracefully + } + }, + member: &model.GrpsIOMember{ + UID: "member-1", + GroupsIOMemberID: func() *int64 { i := int64(12345); return &i }(), + FirstName: "John", + LastName: "Doe", + }, + updates: groupsio.MemberUpdateOptions{ + FirstName: "John", + LastName: "Doe", + }, + }, + { + name: "skip sync when member GroupsIOMemberID is nil", + setupMocks: func() *grpsIOWriterOrchestrator { + return &grpsIOWriterOrchestrator{ + groupsClient: nil, // Could be any value, but GroupsIOMemberID is nil + } + }, + member: &model.GrpsIOMember{ + UID: "member-2", + GroupsIOMemberID: nil, // No member ID - should skip gracefully + FirstName: "Jane", + LastName: "Smith", + }, + updates: groupsio.MemberUpdateOptions{ + FirstName: "Jane", + LastName: "Smith", + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + // Setup + ctx := context.Background() + orchestrator := tc.setupMocks() + + // Execute - should not panic regardless of nil clients or missing data + require.NotPanics(t, func() { + orchestrator.syncMemberToGroupsIO(ctx, tc.member, tc.updates) + }, "syncMemberToGroupsIO should handle nil clients and missing data gracefully") + + // Note: Without log capture or return values, we can only verify no panic occurs. + // The guard clauses at lines 468-471 in grpsio_member_writer.go ensure safe exit. + }) + } +} diff --git a/internal/service/grpsio_service_reader_test.go b/internal/service/grpsio_service_reader_test.go index 153282b..4708fd4 100644 --- a/internal/service/grpsio_service_reader_test.go +++ b/internal/service/grpsio_service_reader_test.go @@ -27,7 +27,7 @@ func TestGrpsIOReaderOrchestratorGetGrpsIOService(t *testing.T) { Type: "primary", UID: testServiceUID, Domain: "lists.testproject.org", - GroupID: 12345, + GroupID: serviceInt64Ptr(12345), Status: "created", GlobalOwners: []string{"admin@testproject.org"}, Prefix: "", @@ -67,7 +67,7 @@ func TestGrpsIOReaderOrchestratorGetGrpsIOService(t *testing.T) { assert.Equal(t, testServiceUID, service.UID) assert.Equal(t, "primary", service.Type) assert.Equal(t, "lists.testproject.org", service.Domain) - assert.Equal(t, int64(12345), service.GroupID) + assert.Equal(t, serviceInt64Ptr(12345), service.GroupID) assert.Equal(t, "created", service.Status) assert.Equal(t, []string{"admin@testproject.org"}, service.GlobalOwners) assert.Equal(t, "", service.Prefix) @@ -155,7 +155,7 @@ func TestGrpsIOReaderOrchestratorGetRevision(t *testing.T) { Type: "primary", UID: testServiceUID, Domain: "lists.testproject.org", - GroupID: 12345, + GroupID: serviceInt64Ptr(12345), Status: "created", ProjectSlug: "test-project", ProjectUID: "test-project-uid", @@ -296,7 +296,7 @@ func TestGrpsIOReaderOrchestratorIntegration(t *testing.T) { Type: "formation", UID: testServiceUID, Domain: "lists.formation.testproject.org", - GroupID: 67890, + GroupID: serviceInt64Ptr(67890), Status: "created", GlobalOwners: []string{"formation@testproject.org", "admin@testproject.org"}, Prefix: "formation", @@ -339,7 +339,7 @@ func TestGrpsIOReaderOrchestratorIntegration(t *testing.T) { // Validate complete service data assert.Equal(t, "formation", service.Type) assert.Equal(t, "lists.formation.testproject.org", service.Domain) - assert.Equal(t, int64(67890), service.GroupID) + assert.Equal(t, serviceInt64Ptr(67890), service.GroupID) assert.Equal(t, "created", service.Status) assert.Equal(t, []string{"formation@testproject.org", "admin@testproject.org"}, service.GlobalOwners) assert.Equal(t, "formation", service.Prefix) @@ -368,3 +368,8 @@ func TestGrpsIOReaderOrchestratorIntegration(t *testing.T) { func serviceStringPtr(s string) *string { return &s } + +// Helper function to create int64 pointer +func serviceInt64Ptr(i int64) *int64 { + return &i +} diff --git a/internal/service/grpsio_service_writer.go b/internal/service/grpsio_service_writer.go index 0b7a852..411e509 100644 --- a/internal/service/grpsio_service_writer.go +++ b/internal/service/grpsio_service_writer.go @@ -11,9 +11,11 @@ import ( "github.com/google/uuid" "github.com/linuxfoundation/lfx-v2-mailing-list-service/internal/domain/model" + "github.com/linuxfoundation/lfx-v2-mailing-list-service/internal/infrastructure/groupsio" "github.com/linuxfoundation/lfx-v2-mailing-list-service/pkg/concurrent" "github.com/linuxfoundation/lfx-v2-mailing-list-service/pkg/constants" "github.com/linuxfoundation/lfx-v2-mailing-list-service/pkg/errors" + "github.com/linuxfoundation/lfx-v2-mailing-list-service/pkg/utils" ) // CreateGrpsIOService creates a new service with transactional operations and rollback @@ -24,7 +26,7 @@ func (sw *grpsIOWriterOrchestrator) CreateGrpsIOService(ctx context.Context, ser "project_uid", service.ProjectUID, ) - // Set service identifiers and timestamps (server-side generation for security) + // Step 1: Set service identifiers and timestamps (server-side generation for security) now := time.Now() service.UID = uuid.New().String() // Always generate server-side, never trust client service.CreatedAt = now @@ -34,14 +36,22 @@ func (sw *grpsIOWriterOrchestrator) CreateGrpsIOService(ctx context.Context, ser var ( keys []string rollbackRequired bool + serviceID *int64 ) defer func() { if err := recover(); err != nil || rollbackRequired { sw.deleteKeys(ctx, keys, true) + + // Clean up GroupsIO resource if created + if serviceID != nil && sw.groupsClient != nil { + if deleteErr := sw.groupsClient.DeleteGroup(ctx, service.GetDomain(), utils.Int64PtrToUint64(serviceID)); deleteErr != nil { + slog.WarnContext(ctx, "failed to cleanup GroupsIO group during rollback", "error", deleteErr, "group_id", *serviceID) + } + } } }() - // Step 1: Validate project exists and populate metadata + // Step 2: Validate project exists and populate metadata if err := sw.validateAndPopulateProject(ctx, service); err != nil { slog.ErrorContext(ctx, "project validation failed", "error", err, @@ -56,7 +66,7 @@ func (sw *grpsIOWriterOrchestrator) CreateGrpsIOService(ctx context.Context, ser "project_name", service.ProjectName, ) - // Step 2: Reserve unique constraints based on service type + // Step 3: Reserve unique constraints based on service type constraintKey, err := sw.reserveUniqueConstraints(ctx, service) if err != nil { rollbackRequired = true @@ -66,7 +76,28 @@ func (sw *grpsIOWriterOrchestrator) CreateGrpsIOService(ctx context.Context, ser keys = append(keys, constraintKey) } - // Step 3: Create service in storage + // Step 4: Create in Groups.io FIRST (if enabled) + groupID, err := sw.createServiceInGroupsIO(ctx, service) + if err != nil { + rollbackRequired = true + return nil, 0, err + } + + if groupID != nil { + // Groups.io creation successful - track for rollback cleanup + serviceID = groupID + + service.GroupID = groupID + service.SyncStatus = "synced" + service.Status = "active" + } else { + // Mock/disabled mode - set appropriate status + service.SyncStatus = "pending" + service.Status = "pending" + slog.InfoContext(ctx, "Groups.io integration disabled - service will be in pending state") + } + + // Step 5: Create service in storage (with Groups.io ID already set) createdService, revision, err := sw.grpsIOWriter.CreateGrpsIOService(ctx, service) if err != nil { slog.ErrorContext(ctx, "failed to create service", @@ -77,24 +108,97 @@ func (sw *grpsIOWriterOrchestrator) CreateGrpsIOService(ctx context.Context, ser rollbackRequired = true return nil, 0, err } - keys = append(keys, createdService.UID) slog.DebugContext(ctx, "service created successfully", "service_uid", createdService.UID, "revision", revision, ) - // Step 4: Publish messages (indexer and access control) + // Step 6: Publish messages (indexer and access control) if sw.publisher != nil { if err := sw.publishServiceMessages(ctx, createdService, model.ActionCreated); err != nil { slog.ErrorContext(ctx, "failed to publish messages", "error", err) - // Don't rollback on message failure, service creation succeeded + // Don't fail the operation on message failure, service creation succeeded } } return createdService, revision, nil } +// createServiceInGroupsIO handles Groups.io group creation and returns the ID +func (sw *grpsIOWriterOrchestrator) createServiceInGroupsIO(ctx context.Context, service *model.GrpsIOService) (*int64, error) { + if sw.groupsClient == nil { + return nil, nil // Skip Groups.io creation + } + + // Use domain methods for effective values + effectiveDomain := service.GetDomain() + effectiveGroupName := service.GetGroupName() + + slog.InfoContext(ctx, "creating group in Groups.io", + "domain", effectiveDomain, + "group_name", effectiveGroupName, + ) + + groupOptions := groupsio.GroupCreateOptions{ + GroupName: effectiveGroupName, + Desc: fmt.Sprintf("Mailing lists for %s", service.ProjectName), // Fixed: was Description + Privacy: "group_privacy_unlisted_public", // Production value + SubGroupAccess: "sub_group_owners", // Production value + EmailDelivery: "email_delivery_none", // Production value + } + + groupResult, err := sw.groupsClient.CreateGroup(ctx, effectiveDomain, groupOptions) + if err != nil { + slog.ErrorContext(ctx, "Groups.io group creation failed", + "error", err, + "domain", effectiveDomain, + "group_name", service.GroupName, + ) + return nil, fmt.Errorf("groups.io creation failed: %w", err) + } + + groupID := int64(groupResult.ID) + slog.InfoContext(ctx, "Groups.io group created successfully", + "group_id", groupResult.ID, + "domain", service.Domain, + ) + + // Step 2: Update group with additional settings that cannot be set at creation time + announce := true + updateOptions := groupsio.GroupUpdateOptions{ + Announce: &announce, + ReplyTo: "group_reply_to_sender", + MembersVisible: "group_view_members_moderators", + CalendarAccess: "group_access_none", + FilesAccess: "group_access_none", + DatabaseAccess: "group_access_none", + WikiAccess: "group_access_none", + PhotosAccess: "group_access_none", + MemberDirectoryAccess: "group_access_moderators_only", + PollsAccess: "polls_access_none", + ChatAccess: "group_access_none", + } + + err = sw.groupsClient.UpdateGroup(ctx, effectiveDomain, uint64(groupID), updateOptions) + if err != nil { + slog.WarnContext(ctx, "Groups.io group update failed, but group creation succeeded", + "error", err, + "group_id", groupID, + "domain", effectiveDomain, + ) + // Don't fail the creation if update fails, as the group was created successfully + // TODO: Will be fixed in next PR to handle the sync status + } else { + slog.InfoContext(ctx, "Groups.io group updated with additional settings", + "group_id", groupID, + "domain", effectiveDomain, + ) + } + + return &groupID, nil +} + // UpdateGrpsIOService updates an existing service with transactional patterns func (sw *grpsIOWriterOrchestrator) UpdateGrpsIOService(ctx context.Context, uid string, service *model.GrpsIOService, expectedRevision uint64) (*model.GrpsIOService, uint64, error) { slog.DebugContext(ctx, "executing update service use case", @@ -191,6 +295,9 @@ func (sw *grpsIOWriterOrchestrator) UpdateGrpsIOService(ctx context.Context, uid "revision", revision, ) + // Sync service updates to Groups.io + sw.syncServiceToGroupsIO(ctx, updatedService) + // Publish update messages if sw.publisher != nil { if err := sw.publishServiceMessages(ctx, updatedService, model.ActionUpdated); err != nil { @@ -432,3 +539,35 @@ func (sw *grpsIOWriterOrchestrator) mergeServiceData(ctx context.Context, existi "mutable_fields", []string{"global_owners", "status", "public"}, ) } + +// syncServiceToGroupsIO handles Groups.io service update with proper error handling +func (sw *grpsIOWriterOrchestrator) syncServiceToGroupsIO(ctx context.Context, service *model.GrpsIOService) { + // Guard clause: skip if Groups.io client not available or service not synced + if sw.groupsClient == nil || service.GroupID == nil { + slog.InfoContext(ctx, "Groups.io integration disabled or service not synced - skipping Groups.io update") + return + } + + // Get domain using helper method + domain, err := sw.getGroupsIODomainForResource(ctx, service.UID, constants.ResourceTypeService) + if err != nil { + slog.WarnContext(ctx, "Groups.io service sync skipped due to domain lookup failure, local update will proceed", + "error", err, "service_uid", service.UID) + return + } + + // Build update options from service model + updates := groupsio.GroupUpdateOptions{ + GlobalOwners: service.GlobalOwners, + } + + // Perform Groups.io service update + err = sw.groupsClient.UpdateGroup(ctx, domain, utils.Int64PtrToUint64(service.GroupID), updates) + if err != nil { + slog.WarnContext(ctx, "Groups.io service update failed, local update will proceed", + "error", err, "domain", domain, "group_id", *service.GroupID) + } else { + slog.InfoContext(ctx, "Groups.io service updated successfully", + "group_id", *service.GroupID, "domain", domain) + } +} diff --git a/internal/service/grpsio_service_writer_test.go b/internal/service/grpsio_service_writer_test.go index f6842ae..bb3fe6b 100644 --- a/internal/service/grpsio_service_writer_test.go +++ b/internal/service/grpsio_service_writer_test.go @@ -33,7 +33,7 @@ func TestGrpsIOWriterOrchestrator_CreateGrpsIOService(t *testing.T) { inputService: &model.GrpsIOService{ Type: "primary", Domain: "lists.test.org", - GroupID: 12345, + GroupID: writerServiceInt64Ptr(12345), GlobalOwners: []string{"admin@test.org"}, Prefix: "", ProjectUID: "project-1", @@ -63,7 +63,7 @@ func TestGrpsIOWriterOrchestrator_CreateGrpsIOService(t *testing.T) { inputService: &model.GrpsIOService{ Type: "formation", Domain: "lists.formation.org", - GroupID: 23456, + GroupID: writerServiceInt64Ptr(23456), GlobalOwners: []string{"admin@formation.org"}, Prefix: "form", ProjectUID: "project-2", @@ -93,7 +93,7 @@ func TestGrpsIOWriterOrchestrator_CreateGrpsIOService(t *testing.T) { inputService: &model.GrpsIOService{ Type: "shared", Domain: "lists.shared.org", - GroupID: 34567, + GroupID: writerServiceInt64Ptr(34567), GlobalOwners: []string{"admin@shared.org"}, Prefix: "", ProjectUID: "project-3", @@ -106,7 +106,7 @@ func TestGrpsIOWriterOrchestrator_CreateGrpsIOService(t *testing.T) { validate: func(t *testing.T, result *model.GrpsIOService, revision uint64, mockRepo *mock.MockRepository) { assert.NotEmpty(t, result.UID) assert.Equal(t, "shared", result.Type) - assert.Equal(t, int64(34567), result.GroupID) + assert.Equal(t, writerInt64Ptr(34567), result.GroupID) assert.False(t, result.Public) assert.Equal(t, "project-3", result.ProjectUID) assert.Equal(t, uint64(1), revision) @@ -122,7 +122,7 @@ func TestGrpsIOWriterOrchestrator_CreateGrpsIOService(t *testing.T) { inputService: &model.GrpsIOService{ Type: "primary", Domain: "lists.test.org", - GroupID: 12345, + GroupID: writerServiceInt64Ptr(12345), GlobalOwners: []string{"admin@test.org"}, Prefix: "", ProjectUID: "nonexistent-project", @@ -147,7 +147,7 @@ func TestGrpsIOWriterOrchestrator_CreateGrpsIOService(t *testing.T) { inputService: &model.GrpsIOService{ Type: "primary", Domain: "lists.audit.org", - GroupID: 45678, + GroupID: writerServiceInt64Ptr(45678), GlobalOwners: []string{"admin@audit.org"}, Writers: []string{"writer1@audit.org", "writer2@audit.org"}, Auditors: []string{"auditor1@audit.org"}, @@ -258,7 +258,7 @@ func TestGrpsIOWriterOrchestrator_CreateGrpsIOService_PublishingErrors(t *testin service := &model.GrpsIOService{ Type: "primary", Domain: "lists.test.org", - GroupID: 12345, + GroupID: writerServiceInt64Ptr(12345), GlobalOwners: []string{"admin@test.org"}, Prefix: "", ProjectUID: "project-1", @@ -648,5 +648,103 @@ func TestGrpsIOWriterOrchestrator_DeleteGrpsIOService_ConflictHandling(t *testin } } +// Helper function to create int64 pointer +func writerServiceInt64Ptr(i int64) *int64 { + return &i +} + +func TestGrpsIOWriterOrchestrator_syncServiceToGroupsIO(t *testing.T) { + testCases := []struct { + name string + setupMock func(*mock.MockRepository) + service *model.GrpsIOService + useNilClient bool + expectedToNotPanic bool + }{ + { + name: "sync with nil GroupsIO client should skip gracefully", + setupMock: func(mockRepo *mock.MockRepository) { + mockRepo.ClearAll() + mockRepo.AddProject("project-1", "test-project", "Test Project") + }, + service: &model.GrpsIOService{ + UID: "service-123", + Type: "primary", + Domain: "lists.test.org", + GroupID: writerServiceInt64Ptr(12345), + GlobalOwners: []string{"admin@test.org", "owner@test.org"}, + ProjectUID: "project-1", + Status: "active", + }, + useNilClient: true, + expectedToNotPanic: true, + }, + { + name: "sync with nil GroupID should skip gracefully", + setupMock: func(mockRepo *mock.MockRepository) { + mockRepo.ClearAll() + }, + service: &model.GrpsIOService{ + UID: "service-123", + Type: "primary", + Domain: "lists.test.org", + GroupID: nil, // Not synced to GroupsIO yet + GlobalOwners: []string{"admin@test.org"}, + ProjectUID: "project-1", + Status: "active", + }, + useNilClient: true, + expectedToNotPanic: true, + }, + { + name: "sync handles domain lookup failure gracefully", + setupMock: func(mockRepo *mock.MockRepository) { + mockRepo.ClearAll() + // Don't add project - this will cause domain lookup to fail + }, + service: &model.GrpsIOService{ + UID: "service-123", + Type: "primary", + Domain: "lists.test.org", + GroupID: writerServiceInt64Ptr(12345), + GlobalOwners: []string{"admin@test.org"}, + ProjectUID: "nonexistent-project", + Status: "active", + }, + useNilClient: true, + expectedToNotPanic: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + // Setup + mockRepo := mock.NewMockRepository() + tc.setupMock(mockRepo) + + grpsIOReader := mock.NewMockGrpsIOReader(mockRepo) + grpsIOWriter := mock.NewMockGrpsIOWriter(mockRepo) + entityReader := mock.NewMockEntityAttributeReader(mockRepo) + + // Create orchestrator without GroupsIO client (simulate mock mode) + orchestrator := NewGrpsIOWriterOrchestrator( + WithGrpsIOWriterReader(grpsIOReader), + WithGrpsIOWriter(grpsIOWriter), + WithEntityAttributeReader(entityReader), + // No WithGroupsIOClient - this simulates mock mode + ) + + // Execute - should not panic + ctx := context.Background() + concreteOrchestrator := orchestrator.(*grpsIOWriterOrchestrator) + + // This should execute without panicking + assert.NotPanics(t, func() { + concreteOrchestrator.syncServiceToGroupsIO(ctx, tc.service) + }, "syncServiceToGroupsIO should handle all error cases gracefully") + }) + } +} + // Note: buildServiceIndexerMessage and buildServiceAccessControlMessage methods are private // and are tested indirectly through the Create/Update/Delete operations above diff --git a/internal/service/grpsio_writer.go b/internal/service/grpsio_writer.go index d88d341..f82b7c6 100644 --- a/internal/service/grpsio_writer.go +++ b/internal/service/grpsio_writer.go @@ -5,10 +5,14 @@ package service import ( "context" + "fmt" "log/slog" "github.com/linuxfoundation/lfx-v2-mailing-list-service/internal/domain/model" "github.com/linuxfoundation/lfx-v2-mailing-list-service/internal/domain/port" + "github.com/linuxfoundation/lfx-v2-mailing-list-service/internal/infrastructure/groupsio" + "github.com/linuxfoundation/lfx-v2-mailing-list-service/pkg/constants" + "github.com/linuxfoundation/lfx-v2-mailing-list-service/pkg/utils" ) // GrpsIOWriter defines the composite interface that combines writers @@ -76,12 +80,20 @@ func WithPublisher(publisher port.MessagePublisher) grpsIOWriterOrchestratorOpti } } +// WithGroupsIOClient sets the GroupsIO client (may be nil for mock/disabled mode) +func WithGroupsIOClient(client groupsio.ClientInterface) grpsIOWriterOrchestratorOption { + return func(w *grpsIOWriterOrchestrator) { + w.groupsClient = client + } +} + // grpsIOWriterOrchestrator orchestrates the service writing process type grpsIOWriterOrchestrator struct { grpsIOWriter port.GrpsIOWriter grpsIOReader port.GrpsIOReader entityReader port.EntityAttributeReader publisher port.MessagePublisher + groupsClient groupsio.ClientInterface // May be nil for mock/disabled mode } // NewGrpsIOWriterOrchestrator creates a new composite writer orchestrator using the option pattern @@ -170,3 +182,116 @@ func (o *grpsIOWriterOrchestrator) deleteKeys(ctx context.Context, keys []string "is_rollback", isRollback, ) } + +// getGroupsIODomainForResource resolves the Groups.io domain for a resource +// Handles both direct service lookup and member -> mailing list -> service lookup chains +// getServiceFromMailingList retrieves the parent service for a mailing list +func (o *grpsIOWriterOrchestrator) getServiceFromMailingList(ctx context.Context, mailingListUID string) (*model.GrpsIOService, error) { + mailingList, _, err := o.grpsIOReader.GetGrpsIOMailingList(ctx, mailingListUID) + if err != nil { + slog.ErrorContext(ctx, "failed to get mailing list for Groups.io domain", "error", err, "mailing_list_uid", mailingListUID) + return nil, err + } + + parentService, _, err := o.grpsIOReader.GetGrpsIOService(ctx, mailingList.ServiceUID) + if err != nil { + slog.ErrorContext(ctx, "failed to get parent service for Groups.io domain", "error", err, "service_uid", mailingList.ServiceUID) + return nil, err + } + + return parentService, nil +} + +func (o *grpsIOWriterOrchestrator) getGroupsIODomainForResource(ctx context.Context, resourceUID string, resourceType string) (string, error) { + switch resourceType { + case constants.ResourceTypeService: + // Direct service lookup + service, _, err := o.grpsIOReader.GetGrpsIOService(ctx, resourceUID) + if err != nil { + slog.ErrorContext(ctx, "failed to get service for Groups.io domain", "error", err, "service_uid", resourceUID) + return "", err + } + return service.Domain, nil + + case constants.ResourceTypeMember: + // Member -> Mailing List -> Service lookup chain + member, _, err := o.grpsIOReader.GetGrpsIOMember(ctx, resourceUID) + if err != nil { + slog.ErrorContext(ctx, "failed to get member for Groups.io domain", "error", err, "member_uid", resourceUID) + return "", err + } + + parentService, err := o.getServiceFromMailingList(ctx, member.MailingListUID) + if err != nil { + return "", err + } + + return parentService.Domain, nil + + case constants.ResourceTypeMailingList: + // Mailing List -> Service lookup + parentService, err := o.getServiceFromMailingList(ctx, resourceUID) + if err != nil { + return "", err + } + + return parentService.Domain, nil + + default: + return "", fmt.Errorf("unsupported resource type: %s", resourceType) + } +} + +// deleteSubgroupWithCleanup handles Groups.io subgroup deletion with proper error handling +func (o *grpsIOWriterOrchestrator) deleteSubgroupWithCleanup(ctx context.Context, serviceUID string, subgroupID *int64) { + // Guard clause: skip if Groups.io client not available or subgroup not synced + if o.groupsClient == nil || subgroupID == nil { + slog.InfoContext(ctx, "Groups.io integration disabled or mailing list not synced - skipping subgroup deletion") + return + } + + // Get domain using helper method + domain, err := o.getGroupsIODomainForResource(ctx, serviceUID, constants.ResourceTypeService) + if err != nil { + slog.WarnContext(ctx, "Groups.io subgroup cleanup skipped due to parent service lookup failure, local deletion will proceed", + "error", err, "service_uid", serviceUID) + return + } + + // Perform Groups.io subgroup deletion + err = o.groupsClient.DeleteSubgroup(ctx, domain, utils.Int64PtrToUint64(subgroupID)) + if err != nil { + slog.WarnContext(ctx, "Groups.io subgroup deletion failed, local deletion will proceed - orphaned subgroups can be cleaned up later", + "error", err, "domain", domain, "subgroup_id", *subgroupID) + } else { + slog.InfoContext(ctx, "Groups.io subgroup deleted successfully", + "subgroup_id", *subgroupID, "domain", domain) + } +} + +// removeMemberFromGroupsIO handles Groups.io member deletion with proper error handling +func (o *grpsIOWriterOrchestrator) removeMemberFromGroupsIO(ctx context.Context, member *model.GrpsIOMember) { + // Guard clause: skip if Groups.io client not available or member not synced + if o.groupsClient == nil || member == nil || member.GroupsIOMemberID == nil { + slog.InfoContext(ctx, "Groups.io integration disabled or member not synced - skipping Groups.io deletion") + return + } + + // Get domain using helper method through member lookup chain + domain, err := o.getGroupsIODomainForResource(ctx, member.UID, constants.ResourceTypeMember) + if err != nil { + slog.WarnContext(ctx, "Groups.io member cleanup skipped due to domain lookup failure, local deletion will proceed", + "error", err, "member_uid", member.UID) + return + } + + // Perform Groups.io member removal + err = o.groupsClient.RemoveMember(ctx, domain, utils.Int64PtrToUint64(member.GroupsIOMemberID)) + if err != nil { + slog.WarnContext(ctx, "Groups.io member deletion failed, local deletion will proceed - orphaned members can be cleaned up later", + "error", err, "domain", domain, "member_id", *member.GroupsIOMemberID) + } else { + slog.InfoContext(ctx, "Groups.io member deleted successfully", + "member_id", *member.GroupsIOMemberID, "domain", domain) + } +} diff --git a/pkg/constants/global.go b/pkg/constants/global.go index a3d3394..3a4eb0b 100644 --- a/pkg/constants/global.go +++ b/pkg/constants/global.go @@ -34,3 +34,13 @@ const ( // EnvNATSCredentials is the environment variable for NATS credentials EnvNATSCredentials = "NATS_CREDENTIALS" ) + +// Resource type constants for domain resolution +const ( + // ResourceTypeService represents a GroupsIO service resource + ResourceTypeService = "service" + // ResourceTypeMember represents a GroupsIO member resource + ResourceTypeMember = "member" + // ResourceTypeMailingList represents a GroupsIO mailing list resource + ResourceTypeMailingList = "mailing_list" +) diff --git a/pkg/constants/validation.go b/pkg/constants/validation.go index f5d7c2a..b60ffc8 100644 --- a/pkg/constants/validation.go +++ b/pkg/constants/validation.go @@ -16,4 +16,4 @@ const ( const ( ErrInvalidTimestampFormat = "invalid timestamp format, expected RFC3339 (2006-01-02T15:04:05Z07:00)" ErrEmptyTimestamp = "timestamp cannot be empty" -) \ No newline at end of file +) diff --git a/pkg/errors/client.go b/pkg/errors/client.go index b27f982..9c836f0 100644 --- a/pkg/errors/client.go +++ b/pkg/errors/client.go @@ -79,3 +79,28 @@ func NewConflict(message string, err ...error) Conflict { }, } } + +// Unauthorized represents an unauthorized error in the application. +type Unauthorized struct { + base +} + +// Error returns the error message for Unauthorized. +func (u Unauthorized) Error() string { + return u.error() +} + +// Unwrap returns the wrapped error, if any. +func (u Unauthorized) Unwrap() error { + return u.err +} + +// NewUnauthorized creates a new Unauthorized error with the provided message. +func NewUnauthorized(message string, err ...error) Unauthorized { + return Unauthorized{ + base: base{ + message: message, + err: errors.Join(err...), + }, + } +} diff --git a/pkg/httpclient/client.go b/pkg/httpclient/client.go new file mode 100644 index 0000000..fa022bc --- /dev/null +++ b/pkg/httpclient/client.go @@ -0,0 +1,219 @@ +// Copyright The Linux Foundation and each contributor to LFX. +// SPDX-License-Identifier: MIT + +package httpclient + +import ( + "context" + "crypto/rand" + "fmt" + "io" + "log/slog" + "math/big" + "net/http" + "strings" + "time" +) + +// RoundTripper interface for request middleware (inspired by production pattern) +type RoundTripper interface { + RoundTrip(req *http.Request, next func(*http.Request) (*http.Response, error)) (*http.Response, error) +} + +// Client represents a generic HTTP client with retry logic and middleware support +type Client struct { + config Config + httpClient *http.Client + roundTrippers []RoundTripper +} + +// Request represents an HTTP request configuration +type Request struct { + Method string + URL string + Headers map[string]string + Body io.Reader +} + +// Response represents an HTTP response +type Response struct { + StatusCode int + Headers http.Header + Body []byte +} + +// RetryableError represents an error that can be retried +type RetryableError struct { + StatusCode int + Message string +} + +func (e *RetryableError) Error() string { + return e.Message +} + +// Do executes an HTTP request with retry logic +func (c *Client) Do(ctx context.Context, req Request) (*Response, error) { + var lastErr error + + for attempt := 0; attempt <= c.config.MaxRetries; attempt++ { + if attempt > 0 { + // Calculate delay with optional exponential backoff + delay := c.config.RetryDelay + if c.config.RetryBackoff { + // Clean doubling with overflow protection + for i := 1; i < attempt && delay < c.config.MaxDelay/2; i++ { + delay *= 2 + } + if delay > c.config.MaxDelay { + delay = c.config.MaxDelay + } + + // Add jitter (25% random variance) to prevent thundering herd + // Use crypto/rand for unpredictable jitter across service restarts + maxJitter := int64(delay / 4) + if maxJitter > 0 { + jitterBig, err := rand.Int(rand.Reader, big.NewInt(maxJitter)) + if err == nil { + delay = delay + time.Duration(jitterBig.Int64()) + } + } + } + + select { + case <-ctx.Done(): + return nil, ctx.Err() + case <-time.After(delay): + } + } + + response, err := c.doRequest(ctx, req) + if err == nil { + return response, nil + } + + lastErr = err + + // Don't retry on certain errors + if !c.shouldRetry(err) { + break + } + } + + slog.ErrorContext(ctx, "request failed", "error", lastErr) + + return nil, lastErr +} + +// doRequest performs a single HTTP request with RoundTripper middleware +func (c *Client) doRequest(ctx context.Context, reqConfig Request) (*Response, error) { + httpReq, err := http.NewRequestWithContext(ctx, reqConfig.Method, reqConfig.URL, reqConfig.Body) + if err != nil { + return nil, fmt.Errorf("failed to create request: %w", err) + } + + // Set default headers + httpReq.Header.Set("Accept", "application/json") + + // Set custom headers + for key, value := range reqConfig.Headers { + httpReq.Header.Set(key, value) + } + + // Execute RoundTripper chain (production pattern) + resp, err := c.executeRoundTripperChain(httpReq, 0) + if err != nil { + return nil, fmt.Errorf("HTTP request failed: %w", err) + } + defer resp.Body.Close() + + body, err := io.ReadAll(resp.Body) + if err != nil { + return nil, fmt.Errorf("failed to read response body: %w", err) + } + + response := &Response{ + StatusCode: resp.StatusCode, + Headers: resp.Header, + Body: body, + } + + // Check for HTTP errors + if resp.StatusCode >= http.StatusBadRequest { + err := &RetryableError{ + StatusCode: resp.StatusCode, + Message: string(body), + } + return response, err + } + + return response, nil +} + +// shouldRetry determines if a request should be retried based on the error +func (c *Client) shouldRetry(err error) bool { + if err == nil { + return false + } + + // Check if it's a retryable error + if retryableErr, ok := err.(*RetryableError); ok { + // Retry on server errors and rate limiting + return retryableErr.StatusCode >= http.StatusInternalServerError || retryableErr.StatusCode == http.StatusTooManyRequests + } + + // Retry on network-related errors + errStr := strings.ToLower(err.Error()) + return strings.Contains(errStr, "timeout") || + strings.Contains(errStr, "connection") || + strings.Contains(errStr, "network") +} + +// Request performs an HTTP request with the specified verb +func (c *Client) Request(ctx context.Context, verb, url string, body io.Reader, headers map[string]string) (*Response, error) { + req := Request{ + Method: verb, + URL: url, + Headers: headers, + Body: body, + } + return c.Do(ctx, req) +} + +// executeRoundTripperChain executes the RoundTripper middleware chain +func (c *Client) executeRoundTripperChain(req *http.Request, index int) (*http.Response, error) { + if index >= len(c.roundTrippers) { + // Base case: execute the actual HTTP request + return c.httpClient.Do(req) + } + + // Execute current RoundTripper with next function + next := func(req *http.Request) (*http.Response, error) { + return c.executeRoundTripperChain(req, index+1) + } + + return c.roundTrippers[index].RoundTrip(req, next) +} + +// AddRoundTripper adds a middleware RoundTripper to the client. +// This method is not safe for concurrent use and should only be called +// during client initialization before making any requests. +func (c *Client) AddRoundTripper(rt RoundTripper) { + c.roundTrippers = append(c.roundTrippers, rt) +} + +// NewClient creates a new HTTP client with the given configuration +func NewClient(config Config) *Client { + // Set sensible default for MaxDelay if not configured + if config.MaxDelay == 0 { + config.MaxDelay = 30 * time.Second + } + + return &Client{ + config: config, + roundTrippers: make([]RoundTripper, 0), + httpClient: &http.Client{ + Timeout: config.Timeout, + }, + } +} diff --git a/pkg/httpclient/client_test.go b/pkg/httpclient/client_test.go new file mode 100644 index 0000000..51253ad --- /dev/null +++ b/pkg/httpclient/client_test.go @@ -0,0 +1,506 @@ +// Copyright The Linux Foundation and each contributor to LFX. +// SPDX-License-Identifier: MIT + +package httpclient + +import ( + "context" + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestNewClient(t *testing.T) { + config := Config{ + Timeout: 10 * time.Second, + MaxRetries: 2, + RetryDelay: 500 * time.Millisecond, + RetryBackoff: true, + } + + client := NewClient(config) + + if client.config.Timeout != config.Timeout { + t.Errorf("Expected timeout %v, got %v", config.Timeout, client.config.Timeout) + } + if client.config.MaxRetries != config.MaxRetries { + t.Errorf("Expected max retries %d, got %d", config.MaxRetries, client.config.MaxRetries) + } + if client.httpClient.Timeout != config.Timeout { + t.Errorf("Expected HTTP client timeout %v, got %v", config.Timeout, client.httpClient.Timeout) + } +} + +func TestClient_Get_Success(t *testing.T) { + // Create a test server + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != "GET" { + t.Errorf("Expected GET request, got %s", r.Method) + } + + w.WriteHeader(http.StatusOK) + _, err := w.Write([]byte(`{"message": "success"}`)) + if err != nil { + t.Errorf("Expected no error, got %v", err) + } + })) + defer server.Close() + + config := Config{ + Timeout: 5 * time.Second, + MaxRetries: 1, + RetryDelay: 100 * time.Millisecond, + RetryBackoff: false, + } + + client := NewClient(config) + ctx := context.Background() + + headers := map[string]string{ + "Custom-Header": "custom-value", + } + + resp, err := client.Request(ctx, "GET", server.URL, nil, headers) + if err != nil { + t.Fatalf("Expected no error, got %v", err) + } + + if resp.StatusCode != http.StatusOK { + t.Errorf("Expected status code 200, got %d", resp.StatusCode) + } + + expectedBody := `{"message": "success"}` + if string(resp.Body) != expectedBody { + t.Errorf("Expected body '%s', got '%s'", expectedBody, string(resp.Body)) + } +} + +func TestClient_Get_NotFound(t *testing.T) { + // Create a test server that returns 404 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusNotFound) + _, err := w.Write([]byte(`{"error": "not found"}`)) + if err != nil { + t.Errorf("Expected no error writing response, got %v", err) + } + })) + defer server.Close() + + config := DefaultConfig() + client := NewClient(config) + ctx := context.Background() + + _, err := client.Request(ctx, "GET", server.URL, nil, nil) + + // Error contract: Non-2xx responses MUST return *RetryableError + // See client.go lines 142-146 where StatusCode >= 400 creates RetryableError + require.Error(t, err, "Expected error for 404 status") + + var retryableErr *RetryableError + require.ErrorAs(t, err, &retryableErr, "Expected *RetryableError for non-2xx response, got %T", err) + assert.Equal(t, http.StatusNotFound, retryableErr.StatusCode, "Expected status code 404") + assert.Contains(t, retryableErr.Message, "not found", "Expected error message to contain 'not found'") +} + +func TestClient_Retry_ServerError(t *testing.T) { + callCount := 0 + + // Create a test server that fails twice then succeeds + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + callCount++ + if callCount <= 2 { + w.WriteHeader(http.StatusInternalServerError) + _, err := w.Write([]byte(`{"error": "server error"}`)) + if err != nil { + t.Errorf("Expected no error, got %v", err) + } + return + } + + w.WriteHeader(http.StatusOK) + _, err := w.Write([]byte(`{"message": "success"}`)) + if err != nil { + t.Errorf("Expected no error, got %v", err) + } + })) + defer server.Close() + + config := Config{ + Timeout: 5 * time.Second, + MaxRetries: 3, + RetryDelay: 10 * time.Millisecond, // Short delay for testing + RetryBackoff: false, + } + + client := NewClient(config) + ctx := context.Background() + + resp, err := client.Request(ctx, "GET", server.URL, nil, nil) + if err != nil { + t.Fatalf("Expected no error after retries, got %v", err) + } + + if resp.StatusCode != http.StatusOK { + t.Errorf("Expected status code 200, got %d", resp.StatusCode) + } + + if callCount != 3 { + t.Errorf("Expected 3 calls (2 failures + 1 success), got %d", callCount) + } +} + +func TestClient_Post(t *testing.T) { + // Create a test server + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != "POST" { + t.Errorf("Expected POST request, got %s", r.Method) + } + + body, _ := io.ReadAll(r.Body) + expectedBody := `{"test": "data"}` + if string(body) != expectedBody { + t.Errorf("Expected body '%s', got '%s'", expectedBody, string(body)) + } + + w.WriteHeader(http.StatusCreated) + _, err := w.Write([]byte(`{"created": true}`)) + if err != nil { + t.Errorf("Expected no error, got %v", err) + } + })) + defer server.Close() + + config := DefaultConfig() + client := NewClient(config) + ctx := context.Background() + + body := strings.NewReader(`{"test": "data"}`) + headers := map[string]string{ + "Content-Type": "application/json", + } + + resp, err := client.Request(ctx, "POST", server.URL, body, headers) + if err != nil { + t.Fatalf("Expected no error, got %v", err) + } + + if resp.StatusCode != http.StatusCreated { + t.Errorf("Expected status code 201, got %d", resp.StatusCode) + } +} + +func TestDefaultConfig(t *testing.T) { + config := DefaultConfig() + + if config.Timeout != 30*time.Second { + t.Errorf("Expected default timeout 30s, got %v", config.Timeout) + } + if config.MaxRetries != 2 { + t.Errorf("Expected default max retries 2, got %d", config.MaxRetries) + } + if config.RetryDelay != 1*time.Second { + t.Errorf("Expected default retry delay 1s, got %v", config.RetryDelay) + } + if !config.RetryBackoff { + t.Error("Expected default retry backoff to be true") + } + if config.MaxDelay != 30*time.Second { + t.Errorf("Expected default max delay 30s, got %v", config.MaxDelay) + } +} + +func TestClient_ExponentialBackoff_DelayCapping(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + })) + defer server.Close() + + config := Config{ + Timeout: 5 * time.Second, + MaxRetries: 5, + RetryDelay: 100 * time.Millisecond, + RetryBackoff: true, + MaxDelay: 500 * time.Millisecond, + } + + client := NewClient(config) + start := time.Now() + + _, err := client.Request(context.Background(), "GET", server.URL, nil, nil) + + elapsed := time.Since(start) + // With exponential backoff: 100ms, 200ms, 400ms, 500ms (capped), 500ms (capped) + // Total expected: ~1.7s + jitter. Allow reasonable range. + if err == nil { + t.Error("Expected error, got nil") + } + if elapsed >= 4*time.Second { + t.Errorf("Request took too long: %v", elapsed) + } + if elapsed <= 1*time.Second { + t.Errorf("Request finished too quickly: %v", elapsed) + } +} + +func TestClient_ExponentialBackoff_JitterVariance(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + })) + defer server.Close() + + config := Config{ + Timeout: 5 * time.Second, + MaxRetries: 3, + RetryDelay: 200 * time.Millisecond, + RetryBackoff: true, + MaxDelay: 1 * time.Second, + } + + // Run multiple attempts to verify jitter adds randomness + var durations []time.Duration + for i := 0; i < 3; i++ { + client := NewClient(config) + start := time.Now() + + _, err := client.Request(context.Background(), "GET", server.URL, nil, nil) + + elapsed := time.Since(start) + durations = append(durations, elapsed) + if err == nil { + t.Error("Expected error, got nil") + } + } + + // Verify durations are different (jitter working) + if durations[0] == durations[1] { + t.Error("Jitter should make durations different") + } + if durations[1] == durations[2] { + t.Error("Jitter should make durations different") + } +} + +func TestNewClient_DefaultMaxDelay(t *testing.T) { + config := Config{ + Timeout: 5 * time.Second, + MaxRetries: 3, + RetryDelay: 100 * time.Millisecond, + RetryBackoff: true, + // MaxDelay not set + } + + client := NewClient(config) + + // Verify default MaxDelay was set + if client.config.MaxDelay != 30*time.Second { + t.Errorf("Expected default MaxDelay 30s, got %v", client.config.MaxDelay) + } +} + +// mockRoundTripper for testing RoundTripper functionality +type mockRoundTripper struct { + called bool + headerSet string + nextCalled bool +} + +func (m *mockRoundTripper) RoundTrip(req *http.Request, next func(*http.Request) (*http.Response, error)) (*http.Response, error) { + m.called = true + + // Add a test header to verify RoundTripper was executed + req.Header.Set("X-Mock-RoundTripper", "executed") + m.headerSet = req.Header.Get("X-Mock-RoundTripper") + + // Call next in chain + resp, err := next(req) + if err == nil { + m.nextCalled = true + } + + return resp, err +} + +// authRoundTripper simulates BasicAuth injection (Groups.io pattern) +type authRoundTripper struct { + username string + password string + called bool +} + +func (a *authRoundTripper) RoundTrip(req *http.Request, next func(*http.Request) (*http.Response, error)) (*http.Response, error) { + a.called = true + + // Inject BasicAuth like Groups.io pattern + if a.username != "" { + req.SetBasicAuth(a.username, a.password) + } + + return next(req) +} + +func TestClient_AddRoundTripper(t *testing.T) { + config := DefaultConfig() + client := NewClient(config) + + mock := &mockRoundTripper{} + client.AddRoundTripper(mock) + + if len(client.roundTrippers) != 1 { + t.Errorf("Expected 1 RoundTripper, got %d", len(client.roundTrippers)) + } +} + +func TestClient_RoundTripper_BasicAuth_Production_Pattern(t *testing.T) { + // Create test server that validates BasicAuth (Groups.io pattern) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + username, password, ok := r.BasicAuth() + + if !ok { + t.Error("Expected BasicAuth to be present") + w.WriteHeader(http.StatusUnauthorized) + return + } + + // Simulate Groups.io token pattern (token as username, empty password) + expectedToken := "jwt.token.here" + if username != expectedToken || password != "" { + t.Errorf("Expected token '%s' with empty password, got '%s':'%s'", expectedToken, username, password) + w.WriteHeader(http.StatusUnauthorized) + return + } + + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`{"message": "authenticated"}`)) + })) + defer server.Close() + + config := DefaultConfig() + client := NewClient(config) + + // Add auth RoundTripper that simulates Groups.io pattern + auth := &authRoundTripper{username: "jwt.token.here", password: ""} + client.AddRoundTripper(auth) + + ctx := context.Background() + resp, err := client.Request(ctx, "GET", server.URL, nil, nil) + + if err != nil { + t.Fatalf("Expected no error, got %v", err) + } + + if resp.StatusCode != http.StatusOK { + t.Errorf("Expected 200 OK, got %d", resp.StatusCode) + } + + if !auth.called { + t.Error("Expected auth RoundTripper to be called") + } +} + +func TestClient_RoundTripper_MultipleMiddleware(t *testing.T) { + // Create test server + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // Verify both RoundTrippers executed + if r.Header.Get("X-Mock-RoundTripper") != "executed" { + t.Errorf("Expected mock RoundTripper header") + } + + // Verify BasicAuth was set + username, password, ok := r.BasicAuth() + if !ok { + t.Error("Expected BasicAuth to be set") + } + if username != "testuser" || password != "testpass" { + t.Errorf("Expected BasicAuth testuser:testpass, got %s:%s", username, password) + } + + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`{"authenticated": true}`)) + })) + defer server.Close() + + config := DefaultConfig() + client := NewClient(config) + + // Add multiple RoundTrippers + mock := &mockRoundTripper{} + auth := &authRoundTripper{username: "testuser", password: "testpass"} + + client.AddRoundTripper(mock) + client.AddRoundTripper(auth) + + ctx := context.Background() + _, err := client.Request(ctx, "GET", server.URL, nil, nil) + + if err != nil { + t.Fatalf("Expected no error, got %v", err) + } + + if !mock.called { + t.Error("Expected mock RoundTripper to be called") + } + + if !auth.called { + t.Error("Expected auth RoundTripper to be called") + } +} + +func TestClient_RoundTripper_WithRetry(t *testing.T) { + attempts := 0 + + // Create test server that fails first time, succeeds second time + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + attempts++ + + // Verify RoundTripper header on every attempt + if r.Header.Get("X-Mock-RoundTripper") != "executed" { + t.Errorf("Expected RoundTripper header on attempt %d", attempts) + } + + if attempts == 1 { + w.WriteHeader(http.StatusInternalServerError) + _, _ = w.Write([]byte(`{"error": "server error"}`)) + return + } + + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`{"status": "ok"}`)) + })) + defer server.Close() + + config := Config{ + Timeout: 5 * time.Second, + MaxRetries: 2, + RetryDelay: 100 * time.Millisecond, + RetryBackoff: false, + } + client := NewClient(config) + + mock := &mockRoundTripper{} + client.AddRoundTripper(mock) + + ctx := context.Background() + resp, err := client.Request(ctx, "GET", server.URL, nil, nil) + + if err != nil { + t.Fatalf("Expected no error, got %v", err) + } + + if resp.StatusCode != http.StatusOK { + t.Errorf("Expected 200 OK, got %d", resp.StatusCode) + } + + if attempts != 2 { + t.Errorf("Expected 2 attempts, got %d", attempts) + } + + if !mock.called { + t.Error("Expected RoundTripper to be called") + } +} diff --git a/pkg/httpclient/config.go b/pkg/httpclient/config.go new file mode 100644 index 0000000..98a66b8 --- /dev/null +++ b/pkg/httpclient/config.go @@ -0,0 +1,37 @@ +// Copyright The Linux Foundation and each contributor to LFX. +// SPDX-License-Identifier: MIT + +package httpclient + +import ( + "time" +) + +// Config holds the configuration for the HTTP client +type Config struct { + // Timeout is the HTTP client timeout for requests + Timeout time.Duration + + // MaxRetries is the maximum number of retry attempts for failed requests + MaxRetries int + + // RetryDelay is the delay between retry attempts + RetryDelay time.Duration + + // RetryBackoff enables exponential backoff for retries + RetryBackoff bool + + // MaxDelay is the maximum delay for exponential backoff (default: 30s) + MaxDelay time.Duration +} + +// DefaultConfig returns a Config with sensible defaults +func DefaultConfig() Config { + return Config{ + Timeout: 30 * time.Second, + MaxRetries: 2, + RetryDelay: 1 * time.Second, + RetryBackoff: true, + MaxDelay: 30 * time.Second, + } +} diff --git a/pkg/utils/time_helpers.go b/pkg/utils/time_helpers.go index d772e0a..817a8ab 100644 --- a/pkg/utils/time_helpers.go +++ b/pkg/utils/time_helpers.go @@ -68,4 +68,4 @@ func FormatTimePtr(t *time.Time) *string { formatted := t.Format(constants.TimestampFormat) return &formatted -} \ No newline at end of file +} diff --git a/pkg/utils/time_helpers_test.go b/pkg/utils/time_helpers_test.go index d240610..dc70ba1 100644 --- a/pkg/utils/time_helpers_test.go +++ b/pkg/utils/time_helpers_test.go @@ -237,4 +237,4 @@ func stringPtr(s string) *string { func timePtr(t time.Time) *time.Time { return &t -} \ No newline at end of file +} diff --git a/pkg/utils/type_converters.go b/pkg/utils/type_converters.go new file mode 100644 index 0000000..9d1e90c --- /dev/null +++ b/pkg/utils/type_converters.go @@ -0,0 +1,40 @@ +// Copyright The Linux Foundation and each contributor to LFX. +// SPDX-License-Identifier: MIT + +// Package utils provides utility functions for the mailing list service. +package utils + +// Int64PtrToUint64 safely converts *int64 to uint64 for API calls. +// Returns 0 if the pointer is nil. +// This is commonly used when converting domain model IDs to API parameter types. +// +// WARNING: Negative values will wrap around to large uint64 values. +// This function assumes the input represents a valid ID (non-negative). +// If you need validation, check for negative values before calling this function. +func Int64PtrToUint64(val *int64) uint64 { + if val == nil { + return 0 + } + // Note: This conversion will wrap negative values. For example: + // -1 becomes 18446744073709551615 (max uint64) + // Callers should ensure IDs from external APIs are non-negative. + return uint64(*val) +} + +// Int64PtrToUint64Ptr safely converts *int64 to *uint64. +// Returns nil if the pointer is nil. +// This is used when optional pointer types need to be converted. +// +// WARNING: Negative values will wrap around to large uint64 values. +// This function assumes the input represents a valid ID (non-negative). +// If you need validation, check for negative values before calling this function. +func Int64PtrToUint64Ptr(val *int64) *uint64 { + if val == nil { + return nil + } + // Note: This conversion will wrap negative values. For example: + // -1 becomes 18446744073709551615 (max uint64) + // Callers should ensure IDs from external APIs are non-negative. + converted := uint64(*val) + return &converted +} diff --git a/pkg/utils/type_converters_test.go b/pkg/utils/type_converters_test.go new file mode 100644 index 0000000..bb283be --- /dev/null +++ b/pkg/utils/type_converters_test.go @@ -0,0 +1,113 @@ +// Copyright The Linux Foundation and each contributor to LFX. +// SPDX-License-Identifier: MIT + +package utils + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestInt64PtrToUint64(t *testing.T) { + tests := []struct { + name string + input *int64 + expected uint64 + }{ + { + name: "nil pointer returns 0", + input: nil, + expected: 0, + }, + { + name: "positive value converts correctly", + input: int64Ptr(123), + expected: 123, + }, + { + name: "zero value converts correctly", + input: int64Ptr(0), + expected: 0, + }, + { + name: "large value converts correctly", + input: int64Ptr(9223372036854775807), // max int64 + expected: 9223372036854775807, + }, + { + name: "negative value wraps around (documented behavior)", + input: int64Ptr(-1), + expected: 18446744073709551615, // uint64 max - documents wrap-around behavior + }, + { + name: "negative large value wraps around", + input: int64Ptr(-9223372036854775808), // min int64 + expected: 9223372036854775808, // wraps to uint64 equivalent + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := Int64PtrToUint64(tt.input) + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestInt64PtrToUint64Ptr(t *testing.T) { + tests := []struct { + name string + input *int64 + expectNil bool + expected uint64 + }{ + { + name: "nil pointer returns nil", + input: nil, + expectNil: true, + }, + { + name: "positive value converts correctly", + input: int64Ptr(456), + expectNil: false, + expected: 456, + }, + { + name: "zero value converts correctly", + input: int64Ptr(0), + expectNil: false, + expected: 0, + }, + { + name: "negative value wraps around (documented behavior)", + input: int64Ptr(-1), + expectNil: false, + expected: 18446744073709551615, // uint64 max - documents wrap-around behavior + }, + { + name: "negative large value wraps around", + input: int64Ptr(-9223372036854775808), // min int64 + expectNil: false, + expected: 9223372036854775808, // wraps to uint64 equivalent + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := Int64PtrToUint64Ptr(tt.input) + + if tt.expectNil { + assert.Nil(t, result) + } else { + assert.NotNil(t, result) + assert.Equal(t, tt.expected, *result) + } + }) + } +} + +// Helper function for tests +func int64Ptr(i int64) *int64 { + return &i +}