From e314979bce8ad12432b026c9ad730de24946f9a6 Mon Sep 17 00:00:00 2001 From: Kris Coleman Date: Thu, 6 Nov 2025 00:19:29 -0500 Subject: [PATCH 1/2] docs: updates after openapi client patterns and further contributor guides updates contributing guide to include development setup, available make commands, new generators, testing, remote operations, ci/cd pipeline, and lefthook setup. it also removes the old contributing guide. updates design docs to describe remote flag synchronization and architecture patterns. Signed-off-by: Kris Coleman --- CONTRIBUTING.md | 122 +++++- DESIGN.md | 43 ++ Makefile | 36 +- README.md | 128 ++++++ docs/openapi-client-pattern.md | 691 +++++++++++++++++++++++++++++++++ 5 files changed, 1008 insertions(+), 12 deletions(-) create mode 100644 docs/openapi-client-pattern.md diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 0ed8726..d745c21 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -2,6 +2,71 @@ Thank you for your interest in contributing to the OpenFeature CLI! This document provides guidelines and instructions to help you get started with contributing to the project. Whether you're fixing a bug, adding a new feature, or improving documentation, your contributions are greatly appreciated. +## Development Setup + +1. **Prerequisites**: + - Go 1.23 or later + - Make + - golangci-lint (will be auto-installed by make commands) + +2. **Clone the repository**: + ```bash + git clone https://github.com/open-feature/cli.git + cd cli + ``` + +3. **Build the project**: + ```bash + make build + ``` + +4. **Run tests**: + ```bash + make test + ``` + +5. **Run all CI checks locally**: + ```bash + make ci + ``` + + This will format code, run linting, execute tests, and verify all generated files are up-to-date. + +## Available Make Commands + +The project includes a comprehensive Makefile with the following commands: + +```bash +make help # Show all available commands +make build # Build the CLI binary +make install # Install the CLI to your system +make lint # Run golangci-lint +make lint-fix # Run golangci-lint with auto-fix +make test # Run unit tests +make test-integration # Run all integration tests +make generate # Generate all code (API clients, docs, schema) +make generate-api # Generate API clients from OpenAPI specs +make generate-docs # Generate documentation +make generate-schema # Generate schema +make verify-generate # Check if all generated files are up to date +make fmt # Format Go code +make ci # Run all CI checks locally (fmt, lint, test, verify-generate) +``` + +### Before Submitting a PR + +Run the following command to ensure your changes will pass CI: + +```bash +make ci +``` + +This command will: +- Format your code +- Run the linter +- Execute all tests +- Verify all generated files are up-to-date + ## Contributing New Generators We welcome contributions for new generators to extend the functionality of the OpenFeature CLI. Below are the steps to contribute a new generator: @@ -62,6 +127,54 @@ make test-csharp-dagger For more information on the integration testing framework, see [Integration Testing](./docs/integration-testing.md). +## Contributing to Remote Operations + +The CLI uses an OpenAPI-driven architecture for remote flag synchronization. If you're contributing to the remote operations (pull/push commands) or API specifications: + +### Modifying the OpenAPI Specification + +1. **Edit the specification**: Update the OpenAPI spec at `api/v0/sync.yaml` +2. **Regenerate the client**: Run `make generate-api` to regenerate the client code +3. **Update the wrapper**: Modify `internal/api/sync/client.go` if needed +4. **Test your changes**: Add or update tests in `internal/api/sync/` + +### Adding New Remote Operations + +1. **Define in OpenAPI**: Add the new operation to `api/v0/sync.yaml` +2. **Regenerate**: Run `make generate-api` +3. **Implement wrapper method**: Add the method in `internal/api/sync/client.go` +4. **Create/update command**: Add or modify commands in `internal/cmd/` +5. **Add tests**: Include unit tests and integration tests +6. **Update documentation**: Update relevant documentation including command docs + +### API Compatibility + +When modifying the OpenAPI specification: + +- **Backwards Compatibility**: Ensure changes don't break existing integrations +- **Versioning**: Use proper API versioning (currently v0) +- **Documentation**: Update the specification descriptions and examples +- **Schema Validation**: Ensure all request/response schemas are properly defined + +For detailed information about the OpenAPI client pattern, see the [OpenAPI Client Pattern documentation](./docs/openapi-client-pattern.md). + +## CI/CD Pipeline + +The project uses GitHub Actions for continuous integration. The following workflows run automatically: + +### PR Validation Workflow +- **Generated Files Check**: Ensures all generated files (OpenAPI client, docs, schema) are up-to-date +- **Format Check**: Verifies all Go code is properly formatted +- **Tests**: Runs all unit tests +- **Integration Tests**: Runs language-specific integration tests + +### PR Test Workflow +- **Unit Tests**: Runs all unit tests +- **Integration Tests**: Runs Dagger-based integration tests for all supported languages + +### PR Lint Workflow +- **golangci-lint**: Runs comprehensive linting with golangci-lint v1.64 + ## Setting Up Lefthook To streamline the setup of Git hooks for this project, we utilize [Lefthook](https://github.com/evilmartians/lefthook). Lefthook automates pre-commit and pre-push checks, ensuring consistent enforcement of best practices across the team. These checks include code formatting, documentation generation, and running tests. @@ -92,8 +205,13 @@ The pre-commit hook is configured to run the following check: The pre-push hook is configured to run the following checks: -1. **Documentation Generation**: Runs `make generate-docs` to ensure documentation is up-to-date. If any changes are detected, the push will be blocked until the changes are committed. -2. **Tests**: Executes `make test` to verify that all tests pass. If any tests fail, the push will be blocked. +1. **Generated Files Check**: Verifies all generated files are up-to-date: + - **OpenAPI Client**: Ensures `internal/api/client/` is current with the OpenAPI spec + - **Documentation**: Ensures `docs/` is current with the latest command structure + - **Schema**: Ensures `schema/` files are up-to-date +2. **Tests**: Executes `make test` to verify that all tests pass + +If any of these checks fail, the push will be blocked. Run `make generate` to update all generated files and commit the changes. ### Running Hooks Manually diff --git a/DESIGN.md b/DESIGN.md index f6f4034..f88885b 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -21,6 +21,7 @@ Benefits of the code generation approach: - **Modular and Extensible Design**: Create a format that allows for future extensions and modularization of flags. - **Language Agnostic**: Support multiple programming languages through a common flag manifest format. - **Provider Independence**: Work with any feature flag provider that can be adapted to the OpenFeature API. +- **Remote Flag Synchronization**: Enable bidirectional synchronization with remote flag management services through a standardized API. ## Non-Goals @@ -28,3 +29,45 @@ Benefits of the code generation approach: - **Validation of Flag Configs**: The project will not initially focus on validating flag configurations for consistency with the flag manifest. - **General-Purpose Configuration**: The project will not aim to create a general-purpose configuration tool for feature flags beyond the scope of the code generation tool. - **Runtime Flag Management**: The CLI is not intended to replace provider SDKs for runtime flag evaluation. + +## Architecture Patterns + +### OpenAPI Client Pattern + +The CLI uses an OpenAPI-driven architecture for all remote operations. This pattern provides several benefits: + +#### Benefits + +1. **Type Safety**: Generated clients from OpenAPI specs ensure compile-time checking of API requests and responses +2. **Self-Documenting**: The OpenAPI specification serves as both implementation guide and documentation +3. **Provider Agnostic**: Any service implementing the Manifest Management API can integrate with the CLI +4. **Maintainability**: Changes to the API are made in one place (the spec) and propagate to all consumers +5. **Extensibility**: New endpoints and operations can be added without breaking existing functionality + +#### Implementation + +The pattern follows this architecture: + +``` +OpenAPI Spec (api/v0/sync.yaml) + ↓ +Generated Client (internal/api/client/*.gen.go) + ↓ +Wrapper Client (internal/api/sync/client.go) + ↓ +CLI Commands (internal/cmd/pull.go, push.go) +``` + +For detailed implementation guidelines, see the [OpenAPI Client Pattern documentation](./docs/openapi-client-pattern.md). + +### Code Generation Pattern + +The CLI generates strongly-typed flag accessors for multiple languages. This pattern: + +1. **Parses** the flag manifest JSON/YAML +2. **Validates** the flag configurations against the schema +3. **Transforms** the data into language-specific structures +4. **Generates** code using Go templates +5. **Formats** the output according to language conventions + +Each generator follows a consistent interface, making it easy to add support for new languages. diff --git a/Makefile b/Makefile index 47902d9..5f583c1 100644 --- a/Makefile +++ b/Makefile @@ -15,9 +15,9 @@ help: @echo " generate-api - Generate API clients from OpenAPI specs" @echo " generate-docs - Generate documentation" @echo " generate-schema - Generate schema" - @echo " verify-generate - Check if generated files are up to date" + @echo " verify-generate - Check if all generated files are up to date" @echo " fmt - Format Go code" - @echo " ci - Run all CI checks locally (lint, test, verify-generate)" + @echo " ci - Run all CI checks locally (fmt, lint, test, verify-generate)" .PHONY: build build: @@ -110,15 +110,31 @@ lint-fix: @echo "Linting with auto-fix completed successfully!" .PHONY: verify-generate -verify-generate: generate - @echo "Checking for uncommitted changes after generation..." - @if [ ! -z "$$(git status --porcelain)" ]; then \ - echo "Error: Generation produced diff. Please run 'make generate' and commit the results."; \ - git diff; \ +verify-generate: + @echo "Checking if all generated files are up-to-date..." + @make generate-api > /dev/null 2>&1 + @if [ ! -z "$$(git diff --name-only internal/api/client/)" ]; then \ + echo "❌ OpenAPI client needs regeneration"; \ + echo " Run: make generate-api"; \ + git diff --stat internal/api/client/; \ exit 1; \ fi - @echo "All generated files are up to date!" + @make generate-docs > /dev/null 2>&1 + @if [ ! -z "$$(git diff --name-only docs/)" ]; then \ + echo "❌ Documentation needs regeneration"; \ + echo " Run: make generate-docs"; \ + git diff --stat docs/; \ + exit 1; \ + fi + @make generate-schema > /dev/null 2>&1 + @if [ ! -z "$$(git diff --name-only schema/)" ]; then \ + echo "❌ Schema needs regeneration"; \ + echo " Run: make generate-schema"; \ + git diff --stat schema/; \ + exit 1; \ + fi + @echo "✅ All generated files are up-to-date!" .PHONY: ci -ci: lint test verify-generate - @echo "All CI checks passed successfully!" \ No newline at end of file +ci: fmt lint test verify-generate + @echo "✅ All CI checks passed successfully!" \ No newline at end of file diff --git a/README.md b/README.md index 7720fd9..50c51ea 100644 --- a/README.md +++ b/README.md @@ -119,6 +119,16 @@ For more advanced usage, read on! The OpenFeature CLI provides the following commands: +| Command | Description | +|---------|-------------| +| `init` | Initialize a new flag manifest | +| `manifest` | Manage flag manifest files (add, list) | +| `compare` | Compare two flag manifests | +| `generate` | Generate strongly typed flag accessors | +| `pull` | Fetch flags from remote sources | +| `push` | Push flags to remote services | +| `version` | Display CLI version | + ### `init` Initialize a new flag manifest in your project. @@ -132,6 +142,55 @@ You can customize the manifest path using configuration options. See [here](./docs/commands/openfeature_init.md) for all available options. +### `manifest` + +Manage flag manifest files with subcommands for adding and listing flags. + +```bash +# Add a new flag interactively +openfeature manifest add + +# Add a boolean flag +openfeature manifest add new-feature --default-value false + +# Add a string flag with description +openfeature manifest add welcome-message \ + --type string \ + --default-value "Hello!" \ + --description "Welcome message for users" + +# List all flags in the manifest +openfeature manifest list +``` + +The manifest command provides: +- **add**: Add new flags to your manifest file +- **list**: Display all flags with their configuration + +See [here](./docs/commands/openfeature_manifest.md) for all available options. + +### `compare` + +Compare two feature flag manifests and display the differences. + +```bash +# Compare your local manifest against another +openfeature compare --against production-flags.json + +# Compare with different output formats +openfeature compare --against other.json --output json +openfeature compare --against other.json --output yaml +openfeature compare --against other.json --output flat +``` + +Output formats: +- **tree**: Hierarchical tree view (default) +- **flat**: Simple flat list +- **json**: JSON format +- **yaml**: YAML format + +See [here](./docs/commands/openfeature_compare.md) for all available options. + ### `generate` Generate strongly typed flag accessors for your project. @@ -163,6 +222,48 @@ openfeature generate typescript --output ./src/flags See [here](./docs/commands/openfeature_generate.md) for all available options. +### `pull` + +Fetch feature flag configurations from a remote source. + +```bash +# Pull flags from a remote API +openfeature pull --flag-source-url https://api.example.com + +# With authentication +openfeature pull --flag-source-url https://api.example.com --auth-token secret-token + +# Pull from a JSON file URL +openfeature pull --flag-source-url https://example.com/flags.json +``` + +The pull command supports: +- HTTP/HTTPS endpoints implementing the OpenFeature Manifest Management API +- Direct JSON/YAML file URLs +- Authentication via bearer tokens + +See [here](./docs/commands/openfeature_pull.md) for all available options. + +### `push` + +Push local flag configurations to a remote flag management service. + +```bash +# Push flags to a remote API +openfeature push --flag-source-url https://api.example.com --auth-token secret-token + +# Dry run to preview changes +openfeature push --flag-source-url https://api.example.com --dry-run +``` + +The push command intelligently: +- Fetches existing flags from the remote +- Compares local flags with remote flags +- Creates new flags that don't exist remotely +- Updates existing flags that have changed + +See [here](./docs/commands/openfeature_push.md) for all available options. + ### `version` Print the version number of the OpenFeature CLI. @@ -205,6 +306,33 @@ The flag manifest file should follow the [JSON schema](https://raw.githubusercon } ``` +## Remote Flag Management + +The OpenFeature CLI supports synchronizing flags with remote flag management services through a standardized OpenAPI-based approach. This enables teams to: + +- **Pull flags** from centralized flag management systems +- **Push flags** back to maintain consistency across environments +- **Integrate** with any service that implements the Manifest Management API + +### OpenAPI Client Pattern + +The CLI uses an OpenAPI-driven architecture for remote operations: + +1. **Standardized API**: All remote operations conform to the [Manifest Management API](./api/v0/sync.yaml) OpenAPI specification +2. **Type-Safe Clients**: Generated clients provide compile-time safety and better IDE support +3. **Provider Agnostic**: Any service implementing the API specification can integrate with the CLI + +For detailed information about implementing or extending the OpenAPI client pattern, see the [OpenAPI Client Pattern documentation](./docs/openapi-client-pattern.md). + +### Implementing the Manifest Management API + +If you're building a flag management service that needs to integrate with the OpenFeature CLI, implement the endpoints defined in the [sync.yaml](./api/v0/sync.yaml) specification: + +- `GET /openfeature/v0/manifest` - Retrieve the project manifest +- `POST /openfeature/v0/manifest/flags` - Create new flags +- `PUT /openfeature/v0/manifest/flags/{key}` - Update existing flags +- `DELETE /openfeature/v0/manifest/flags/{key}` - Archive/delete flags + ## Configuration The OpenFeature CLI uses an optional configuration file to override default settings and customize behavior. diff --git a/docs/openapi-client-pattern.md b/docs/openapi-client-pattern.md new file mode 100644 index 0000000..57cf2fc --- /dev/null +++ b/docs/openapi-client-pattern.md @@ -0,0 +1,691 @@ +# OpenAPI Client Pattern for Remote Operations + +This document describes the OpenAPI client generation pattern used in the OpenFeature CLI for implementing remote API operations. This pattern provides type-safe, well-documented client code for interacting with remote services. + +## Overview + +The OpenFeature CLI uses a code generation approach for remote API operations: + +1. **Define**: Remote API functionality is defined in an OpenAPI specification (YAML) +2. **Generate**: A type-safe Go client is generated from the specification +3. **Wrap**: A thin wrapper provides convenience methods and integrates with CLI infrastructure +4. **Use**: Commands consume the wrapped client for all remote operations + +This pattern ensures consistency, type safety, and maintainability across all remote operations in the CLI. + +## Current Implementation + +The current implementation consists of: + +- **OpenAPI Specification**: `api/v0/sync.yaml` - Defines the Manifest Management API v0 +- **Generated Client**: `internal/api/client/sync_client.gen.go` - Auto-generated from the spec +- **Wrapper Client**: `internal/api/sync/client.go` - Provides convenience methods and retry logic +- **Test Suite**: `internal/api/sync/retry_test.go` - Tests retry logic and error handling +- **CLI Commands**: `internal/cmd/pull.go` and `internal/cmd/push.go` - Use the wrapped client + +### Dependencies + +The implementation relies on these key libraries: + +- **[oapi-codegen](https://github.com/oapi-codegen/oapi-codegen)**: Generates type-safe Go clients from OpenAPI specs +- **[GoRetry](https://github.com/kriscoleman/GoRetry)**: Provides retry logic with exponential backoff for transient failures +- **[gock](https://github.com/h2non/gock)**: HTTP mocking for comprehensive testing without real servers + +## Architecture + +``` +┌─────────────────────┐ +│ OpenAPI Spec │ +│ (api/v0/sync.yaml) │ +└──────────┬──────────┘ + │ + │ oapi-codegen + ▼ +┌─────────────────────┐ +│ Generated Client │ +│ (internal/api/ │ +│ client/*.gen.go) │ +└──────────┬──────────┘ + │ + │ wrapped by + ▼ +┌─────────────────────┐ +│ Sync Client │ +│ (internal/api/ │ +│ sync/client.go) │ +└──────────┬──────────┘ + │ + │ used by + ▼ +┌─────────────────────┐ +│ CLI Commands │ +│ (internal/cmd/ │ +│ pull.go,push.go) │ +└─────────────────────┘ +``` + +## Benefits + +### 1. Type Safety + +- Compile-time checking of API requests and responses +- Eliminates string-based URL construction errors +- Strongly typed request/response models + +### 2. Documentation as Code + +- API contract is clearly defined in OpenAPI spec +- Self-documenting code through generated types +- Examples and descriptions in the spec translate to code comments + +### 3. Maintainability + +- Changes to API are made in one place (the spec) +- Regenerate client to update all consumers +- Clear separation between generated and custom code + +### 4. Consistency + +- All remote operations follow the same pattern +- Standardized error handling across endpoints +- Uniform logging and debugging capabilities + +### 5. Extensibility + +- Easy to add new endpoints (just update the spec) +- Supports multiple API versions side-by-side +- Foundation for plugin systems and provider implementations + +## Implementation Guide + +### Step 1: Define the OpenAPI Specification + +Create or update the OpenAPI specification in `api/v0/` directory: + +```yaml +# api/v0/sync.yaml +openapi: 3.0.3 +info: + title: Manifest Management API + version: 1.0.0 + description: | + CRUD endpoints for managing feature flags through the OpenFeature CLI. + +paths: + /openfeature/v0/manifest: + get: + summary: Get Project Manifest + description: Returns the project manifest containing active flags + security: + - bearerAuth: [] + responses: + '200': + description: Manifest exported successfully + content: + application/json: + schema: + $ref: '#/components/schemas/ManifestEnvelope' +``` + +**Best Practices:** + +- Use semantic versioning for the API version +- Include detailed descriptions for all operations +- Define reusable schemas in `components/schemas` +- Specify all possible response codes +- Document authentication requirements + +### Step 2: Generate the Client + +Add a Makefile target to generate the client: + +```makefile +# Generate OpenAPI clients +generate-sync-client: + oapi-codegen -package syncclient \ + -generate types,client \ + -o internal/api/client/sync_client.gen.go \ + api/v0/sync.yaml +``` + +Run the generator: + +```bash +make generate-sync-client +``` + +This produces type-safe Go code in `internal/api/client/sync_client.gen.go` including: + +- Request/response types +- Client interface +- HTTP request builders +- Response parsers + +**Important:** Never manually edit generated files. Always regenerate from the spec. + +### Step 3: Create a Wrapper Client + +Create a wrapper in `internal/api/sync/client.go` that: + +- Provides convenience methods +- Handles common concerns (logging, retries, error mapping) +- Converts between API and internal models + +```go +// internal/api/sync/client.go +package sync + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + + syncclient "github.com/open-feature/cli/internal/api/client" + "github.com/open-feature/cli/internal/flagset" + "github.com/open-feature/cli/internal/logger" +) + +// Client wraps the generated OpenAPI client with convenience methods +type Client struct { + apiClient *syncclient.ClientWithResponses + authToken string +} + +// NewClient creates a new sync client +func NewClient(baseURL string, authToken string) (*Client, error) { + // Configure HTTP client options + var opts []syncclient.ClientOption + + if authToken != "" { + opts = append(opts, syncclient.WithRequestEditorFn( + func(ctx context.Context, req *http.Request) error { + req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", authToken)) + return nil + })) + } + + apiClient, err := syncclient.NewClientWithResponses(baseURL, opts...) + if err != nil { + return nil, fmt.Errorf("failed to create API client: %w", err) + } + + return &Client{ + apiClient: apiClient, + authToken: authToken, + }, nil +} + +// PullFlags fetches flags from the remote API (without retry logic) +func (c *Client) PullFlags(ctx context.Context) (*flagset.Flagset, error) { + logger.Default.Debug("Fetching flags using sync API client") + + resp, err := c.apiClient.GetOpenfeatureV0ManifestWithResponse(ctx) + if err != nil { + return nil, fmt.Errorf("failed to fetch manifest: %w", err) + } + + // Debug logging + if resp.HTTPResponse != nil { + logger.Default.Debug(fmt.Sprintf("Pull response: HTTP %d", + resp.HTTPResponse.StatusCode)) + } + + // Handle error responses + if resp.HTTPResponse.StatusCode < 200 || resp.HTTPResponse.StatusCode >= 300 { + if resp.JSON401 != nil { + return nil, fmt.Errorf("authentication failed: %s", + resp.JSON401.Error.Message) + } + // ... handle other error cases + } + + // Convert API model to internal model + return convertToFlagset(resp.JSON200) +} +``` + +**Key responsibilities of the wrapper:** + +- Authentication/authorization setup +- Request/response logging for debugging +- Error handling and user-friendly error messages +- Model conversion (API types ↔ internal types) +- Retry logic for write operations (POST/PUT/DELETE) using GoRetry library +- Context propagation +- Smart push logic (comparing local vs remote flags before making changes) + +The wrapper also defines result types for operations: + +```go +// PushResult tracks changes made during a push operation +type PushResult struct { + Created []flagset.Flag // Flags created on the remote + Updated []flagset.Flag // Flags updated on the remote + Unchanged []flagset.Flag // Flags that didn't need changes +} +``` + +### Step 4: Use in Commands + +Update commands to use the wrapped client: + +```go +// internal/cmd/pull.go +func GetPullCmd() *cobra.Command { + return &cobra.Command{ + Use: "pull", + Short: "Pull flag configurations from a remote source", + RunE: func(cmd *cobra.Command, args []string) error { + flagSourceUrl := config.GetFlagSourceUrl(cmd) + authToken := config.GetAuthToken(cmd) + + // Use the sync API client + flags, err := manifest.LoadFromSyncAPI(flagSourceUrl, authToken) + if err != nil { + return fmt.Errorf("error fetching flags: %w", err) + } + + // Process flags... + return nil + }, + } +} +``` + +## URL Handling + +The generated client expects the **base URL only** (domain and optionally port). The client automatically constructs full paths from the OpenAPI spec. + +**Correct:** + +```bash +# Base URL only +openfeature pull --flag-source-url https://api.example.com +openfeature pull --flag-source-url https://api.example.com:8080 +``` + +**Incorrect:** + +```bash +# Don't include API paths in the URL +openfeature pull --flag-source-url https://api.example.com/openfeature/v0/manifest +``` + +The generated client automatically: + +- Adds a trailing slash to the base URL +- Constructs operation paths from the spec (e.g., `/openfeature/v0/manifest`) +- Handles path parameters and query strings + +## Error Handling + +The pattern provides multiple levels of error handling: + +### 1. Network Errors + +```go +resp, err := c.apiClient.GetOpenfeatureV0ManifestWithResponse(ctx) +if err != nil { + return nil, fmt.Errorf("failed to fetch manifest: %w", err) +} +``` + +### 2. HTTP Status Errors + +```go +if resp.HTTPResponse.StatusCode >= 400 { + // Access typed error responses + if resp.JSON401 != nil { + return nil, fmt.Errorf("authentication failed: %s", + resp.JSON401.Error.Message) + } +} +``` + +### 3. Schema Validation + +The generated client automatically validates responses against the OpenAPI schema. + +## Debugging + +Enable debug logging to see full HTTP request/response details: + +```bash +openfeature pull --flag-source-url https://api.example.com --debug +``` + +The wrapper client should log: + +- Request URLs and headers +- Request bodies (sanitized) +- Response status codes +- Response bodies (truncated if large) + +Example debug output: + +``` +[DEBUG] Loading flags from sync API at https://api.example.com +[DEBUG] Pull response: HTTP 200 - OK +[DEBUG] Response body: {"flags":[{"key":"feature-x",...}]} +[DEBUG] Successfully pulled 5 flags +``` + +## Testing + +### HTTP Mocking with Gock + +The CLI uses [gock](https://github.com/h2non/gock) for mocking HTTP requests in tests. This provides a clean way to test retry logic, error handling, and different response scenarios without needing a real server. + +**Why Gock?** +- **Simple API**: Fluent interface for setting up mock responses +- **Request Matching**: Can match on URL, headers, body, query params +- **Response Stacking**: Queue multiple responses for testing retry logic +- **Network Interception**: Works at the HTTP transport level, no code changes needed +- **Clean Test Isolation**: Each test can set up its own mock expectations + +```go +// internal/api/sync/retry_test.go +import "github.com/h2non/gock" + +func TestRetryLogic(t *testing.T) { + defer gock.Off() // Clean up after test + + // First attempt: 500 error (will trigger retry) + gock.New("https://api.example.com"). + Post("/openfeature/v0/manifest/flags"). + Reply(500). + JSON(map[string]interface{}{ + "error": map[string]interface{}{ + "message": "Internal Server Error", + "status": 500, + }, + }) + + // Second attempt: Success + gock.New("https://api.example.com"). + Post("/openfeature/v0/manifest/flags"). + Reply(201). + JSON(map[string]interface{}{ + "flag": map[string]interface{}{ + "key": "test-flag", + "type": "boolean", + "defaultValue": true, + }, + "updatedAt": "2024-03-02T09:45:03.000Z", + }) + + // Create client and test retry behavior + client, _ := NewClient("https://api.example.com", "test-token") + + // This will fail once, then succeed on retry + result, err := client.PushFlags(ctx, localFlags, remoteFlags, false) + assert.NoError(t, err) + assert.Len(t, result.Created, 1) +} +``` + +**Key Testing Patterns with Gock:** + +1. **Testing Retry Logic**: Stack multiple responses for the same endpoint to simulate failures followed by success +2. **Testing Error Handling**: Mock specific HTTP status codes and error responses +3. **Request Validation**: Use `MatchBody()` to verify request payloads +4. **Header Validation**: Use `MatchHeader()` to ensure authentication headers are sent + +```go +// Testing authentication header +gock.New("https://api.example.com"). + Post("/openfeature/v0/manifest/flags"). + MatchHeader("Authorization", "Bearer test-token"). + Reply(201). + JSON(successResponse) + +// Testing request body +gock.New("https://api.example.com"). + Post("/openfeature/v0/manifest/flags"). + MatchType("json"). + BodyString(`{"key":"test-flag","type":"boolean","defaultValue":true}`). + Reply(201). + JSON(successResponse) +``` + +### When to Use Gock vs Interface Mocking + +**Use Gock when:** +- Testing retry logic and transient failures +- Testing HTTP-specific behaviors (status codes, headers, timeouts) +- Testing the full request/response cycle +- Verifying exact request payloads and headers + +**Use Interface Mocking when:** +- Testing business logic independent of HTTP +- Need fast unit tests without network overhead +- Testing complex data transformations +- Want to avoid HTTP transport complexity + +### Unit Tests + +For testing business logic without HTTP calls, you can also mock the generated client interface: + +```go +// client_test.go +type mockSyncClient struct { + getManifestFunc func(ctx context.Context) (*GetOpenfeatureV0ManifestResponse, error) +} + +func (m *mockSyncClient) GetOpenfeatureV0ManifestWithResponse(ctx context.Context, + reqEditors ...RequestEditorFn) (*GetOpenfeatureV0ManifestResponse, error) { + return m.getManifestFunc(ctx) +} + +func TestPullFlags(t *testing.T) { + mock := &mockSyncClient{ + getManifestFunc: func(ctx context.Context) (*GetOpenfeatureV0ManifestResponse, error) { + return &GetOpenfeatureV0ManifestResponse{ + JSON200: &ManifestEnvelope{ + Flags: []ManifestFlag{{Key: "test-flag"}}, + }, + }, nil + }, + } + + // Test with mock client... +} +``` + +### Integration Tests + +Test against a real implementation of the API: + +```bash +# Run integration tests +make test-integration + +# Run specific language integration tests +make test-integration-go +make test-integration-csharp +make test-integration-nodejs +``` + +## Future Extensions + +### Plugin System + +This pattern enables a plugin architecture for supporting multiple flag providers: + +```go +// Provider interface using the pattern +type Provider interface { + PullFlags(ctx context.Context) (*flagset.Flagset, error) + PushFlags(ctx context.Context, flags *flagset.Flagset) error +} + +// Each provider implements the interface +type FlagdStudioProvider struct { + client *sync.Client +} + +type LaunchDarklyProvider struct { + client *launchdarkly.Client +} + +// Register providers +providers := map[string]Provider{ + "flagd-studio": NewFlagdStudioProvider(config), + "launchdarkly": NewLaunchDarklyProvider(config), +} +``` + +### Multiple API Versions + +Support multiple API versions simultaneously: + +``` +api/ +├── v0/ +│ └── sync.yaml +└── v1/ + └── sync.yaml + +internal/api/client/ +├── v0/ +│ └── sync_client.gen.go +└── v1/ + └── sync_client.gen.go +``` + +### Custom Operations + +Add provider-specific operations while maintaining the core pattern: + +```go +// Extended client for provider-specific features +type FlagdStudioClient struct { + *sync.Client +} + +func (c *FlagdStudioClient) ListEnvironments(ctx context.Context) ([]Environment, error) { + // Provider-specific operation +} +``` + +## Common Patterns + +### Retry Logic + +The `PushFlags` method uses the [GoRetry library](https://github.com/kriscoleman/GoRetry) to handle transient failures when creating or updating flags: + +```go +import goretry "github.com/kriscoleman/GoRetry" + +func (c *Client) PushFlags(ctx context.Context, localFlags *flagset.Flagset, remoteFlags *flagset.Flagset, dryRun bool) (*PushResult, error) { + // ... flag comparison logic ... + + for _, flag := range toCreate { + err := goretry.IfNeededWithContext(ctx, func(ctx context.Context) error { + body, err := c.convertFlagToAPIBody(flag) + if err != nil { + return fmt.Errorf("failed to convert flag %s: %w", flag.Key, err) + } + + resp, err := c.apiClient.PostOpenfeatureV0ManifestFlagsWithResponse(ctx, body) + if err != nil { + return fmt.Errorf("failed to create flag %s: %w", flag.Key, err) + } + + if resp.HTTPResponse.StatusCode >= 500 { + return &httpError{statusCode: resp.HTTPResponse.StatusCode} // Retryable + } + + return nil + }, goretry.WithTransientErrorFunc(isTransientHTTPError)) + + // ... handle result ... + } +} +``` + +Note: Retry logic is only used for write operations (POST, PUT, DELETE) where transient failures are more impactful. Read operations like `PullFlags` don't implement retry logic by default. + +### Pagination + +```go +func (c *Client) ListAllFlags(ctx context.Context) ([]*flagset.Flag, error) { + var allFlags []*flagset.Flag + cursor := "" + + for { + // Note: Pagination is not yet implemented in the current API spec + // This is an example of how it could work + resp, err := c.apiClient.GetOpenfeatureV0ManifestWithResponse(ctx, + &GetOpenfeatureV0ManifestParams{Cursor: &cursor}) + if err != nil { + return nil, err + } + + allFlags = append(allFlags, convertFlags(resp.JSON200.Flags)...) + + // Check for pagination cursor in response + if resp.JSON200.NextCursor == nil { + break + } + cursor = *resp.JSON200.NextCursor + } + + return allFlags, nil +} +``` + +### Streaming + +```go +func (c *Client) StreamFlags(ctx context.Context) (<-chan *flagset.Flag, error) { + ch := make(chan *flagset.Flag) + + go func() { + defer close(ch) + + // Server-sent events or WebSocket connection + stream, err := c.apiClient.StreamManifest(ctx) + if err != nil { + return + } + + for event := range stream { + ch <- convertFlag(event) + } + }() + + return ch, nil +} +``` + +## Checklist for Adding a New Remote Operation + +- [ ] Update OpenAPI specification in `api/v0/` +- [ ] Regenerate client: `make generate-api` +- [ ] Add wrapper method in `internal/api/sync/client.go` +- [ ] Add debug logging for requests/responses +- [ ] Handle all error cases (4xx, 5xx) +- [ ] Convert API models to internal models +- [ ] Add unit tests with mocked client +- [ ] Add integration tests +- [ ] Update command to use new operation +- [ ] Update documentation +- [ ] Test with `--debug` flag + +## References + +- [OpenAPI Specification](https://spec.openapis.org/oas/v3.0.3) +- [oapi-codegen Documentation](https://github.com/oapi-codegen/oapi-codegen) +- [Sync API Specification](../../api/v0/sync.yaml) + +## Related Patterns + +- **Code Generation Pattern**: Used for generating flag accessors (see [DESIGN.md](../DESIGN.md)) +- **Provider Pattern**: Abstraction for different flag provider implementations +- **Repository Pattern**: Separates data access logic from business logic + +--- + +For questions or suggestions about this pattern, please open an issue or discussion in the GitHub repository. From fb40fa2d94fcb2992ff50447c3db5d6805fd2f39 Mon Sep 17 00:00:00 2001 From: Kris Coleman Date: Mon, 17 Nov 2025 12:51:19 -0500 Subject: [PATCH 2/2] ci: adds formatting and generated files checks (#180) Adds a go formatting check to the CI pipeline to ensure code style consistency. Also, adds a check to validate that generated files (openapi client, docs, schema) are up-to-date before allowing a PR to be merged. This prevents merge conflicts and ensures that the generated code is always current. Signed-off-by: Kris Coleman --- .github/workflows/pr-lint.yml | 20 ++++++++++++++++ .github/workflows/pr-test.yml | 43 +++++++++++++++++++++++++++++++---- lefthook.yml | 25 +++++++++++++++++--- 3 files changed, 80 insertions(+), 8 deletions(-) diff --git a/.github/workflows/pr-lint.yml b/.github/workflows/pr-lint.yml index cee415f..fafc73d 100644 --- a/.github/workflows/pr-lint.yml +++ b/.github/workflows/pr-lint.yml @@ -15,6 +15,26 @@ permissions: checks: write jobs: + format-check: + name: check formatting + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + - uses: actions/setup-go@v5 + with: + go-version-file: 'go.mod' + - name: Check Go formatting + run: | + unformatted=$(go fmt ./...) + if [ -n "$unformatted" ]; then + echo "::error ::The following files need formatting. Please run 'go fmt ./...' or 'make fmt'" + echo "$unformatted" + exit 1 + fi + echo "✅ All Go files are properly formatted" + golangci: name: lint runs-on: ubuntu-latest diff --git a/.github/workflows/pr-test.yml b/.github/workflows/pr-test.yml index e9dfec9..e593e37 100644 --- a/.github/workflows/pr-test.yml +++ b/.github/workflows/pr-test.yml @@ -24,8 +24,8 @@ jobs: - name: Run tests run: go test ./... - docs-check: - name: Validate docs + generated-files-check: + name: Validate generated files runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 @@ -36,11 +36,44 @@ jobs: with: go-version-file: 'go.mod' - - run: make generate-docs - - name: Check no diff + - name: Generate OpenAPI client + run: make generate-api + - name: Check OpenAPI client is up-to-date + run: | + if [ ! -z "$(git status --porcelain internal/api/client/)" ]; then + echo "::error ::OpenAPI client generation produced diff. Run 'make generate-api' and commit results." + git diff internal/api/client/ + exit 1 + fi + + - name: Generate documentation + run: make generate-docs + - name: Check documentation is up-to-date + run: | + if [ ! -z "$(git status --porcelain docs/)" ]; then + echo "::error ::Documentation generation produced diff. Run 'make generate-docs' and commit results." + git diff docs/ + exit 1 + fi + + - name: Generate schema + run: make generate-schema + - name: Check schema is up-to-date + run: | + if [ ! -z "$(git status --porcelain schema/)" ]; then + echo "::error ::Schema generation produced diff. Run 'make generate-schema' and commit results." + git diff schema/ + exit 1 + fi + + - name: Final check for any uncommitted changes run: | if [ ! -z "$(git status --porcelain)" ]; then - echo "::error file=Makefile::Doc generation produced diff. Run 'make generate-docs' and commit results." + echo "::error ::Generation produced uncommitted changes. Please run 'make generate' and commit all changes." + echo "Changed files:" + git status --porcelain + echo "" + echo "Diff:" git diff exit 1 fi diff --git a/lefthook.yml b/lefthook.yml index 758bdac..f6d6f86 100644 --- a/lefthook.yml +++ b/lefthook.yml @@ -8,13 +8,32 @@ pre-commit: stage_fixed: true pre-push: commands: - generate-docs: + check-generated-files: run: | + echo "Checking all generated files are up-to-date..." + + # Check OpenAPI client + make generate-api + if ! git diff --quiet internal/api/client/; then + echo "❌ OpenAPI client is outdated. Please run 'make generate-api' and commit the changes." + exit 1 + fi + + # Check documentation make generate-docs - if ! git diff --quiet; then - echo "Documentation is outdated. Please run 'make generate-docs' and commit the changes." + if ! git diff --quiet docs/; then + echo "❌ Documentation is outdated. Please run 'make generate-docs' and commit the changes." + exit 1 + fi + + # Check schema + make generate-schema + if ! git diff --quiet schema/; then + echo "❌ Schema is outdated. Please run 'make generate-schema' and commit the changes." exit 1 fi + + echo "✅ All generated files are up-to-date" skip: false tests: run: make test