diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md new file mode 100644 index 00000000..052e8930 --- /dev/null +++ b/.github/pull_request_template.md @@ -0,0 +1,64 @@ +## Change Description + + +## Change Characteristics + +- [ ] **This PR contains beta functionality** +- [ ] **This PR requires introduction of breaking changes** + +## Checklist + + +_All full (or complete) PRs that need review prior to merge should have the following box checked._ + +_If contributing a partial or incomplete change (expecting the development team to complete the remaining work) please leave the box unchecked_ + +- [ ] **Check to confirm**: I have performed a review of my PR against the [PR checklist](../contributing/pr-checklist.md) and confirm that: + - Changes have proper unit and acceptance test coverage (including regression tests) + - Impacted command, parameter and flag descriptions have been reviewed and updated + - Impacted command examples have been reviewed and updated + - Impacted documentation files have been reviewed and updated + - Does not introduce breaking changes to commands, parameters or flags (unless required to do so) + - I am aware that changes to generated code may not be merged + +## Required SDK Upgrades + + + + +## Testing + +This PR has been tested with: + +- [ ] Unit tests _(please paste commands and results below)_ +- [ ] Acceptance tests _(please paste commands and results below)_ +- [ ] End-to-end tests _(please paste the link to the actions workflow runs)_ +- [ ] Not applicable _(no evidences needed)_ + +### Shell Command(s) + +```shell + +``` + +### Testing Results + + +
+ Expand Results + +```shell + +``` + +
+ +### End-to-end Tests Workflow Links + + +- \ No newline at end of file diff --git a/.vscode/extensions.json b/.vscode/extensions.json new file mode 100644 index 00000000..5482fc7c --- /dev/null +++ b/.vscode/extensions.json @@ -0,0 +1,28 @@ +{ + "recommendations": [ + // Go development + "golang.go", + // Code quality and linting + "golangci.golangci-lint", + // Protocol Buffers support + "zxh404.vscode-proto3", + // YAML support for configuration files + "redhat.vscode-yaml", + // Makefile support + "ms-vscode.makefile-tools", + // Docker support for container testing + "ms-azuretools.vscode-docker", + // Markdown support for documentation + "yzhang.markdown-all-in-one", + "davidanson.vscode-markdownlint", + // JSON support with schema validation + "ms-vscode.vscode-json", + // Code spell checker + "streetsidesoftware.code-spell-checker" + ], + "unwantedRecommendations": [ + // Avoid conflicting Go extensions + "ms-vscode.go", + "lukehoban.go" + ] +} \ No newline at end of file diff --git a/.vscode/launch.json b/.vscode/launch.json new file mode 100644 index 00000000..f760af0a --- /dev/null +++ b/.vscode/launch.json @@ -0,0 +1,101 @@ +{ + "version": "0.2.0", + "configurations": [ + { + "name": "Launch pingcli", + "type": "go", + "request": "launch", + "mode": "auto", + "program": "${workspaceFolder}", + "args": [ + "--help" + ], + "console": "integratedTerminal" + }, + { + "name": "Debug config command", + "type": "go", + "request": "launch", + "mode": "auto", + "program": "${workspaceFolder}", + "args": [ + "config", + "--help" + ], + "console": "integratedTerminal" + }, + { + "name": "Debug platform export", + "type": "go", + "request": "launch", + "mode": "auto", + "program": "${workspaceFolder}", + "args": [ + "platform", + "export", + "--help" + ], + "console": "integratedTerminal" + }, + { + "name": "Debug request command", + "type": "go", + "request": "launch", + "mode": "auto", + "program": "${workspaceFolder}", + "args": [ + "request", + "--help" + ], + "console": "integratedTerminal" + }, + { + "name": "Debug version", + "type": "go", + "request": "launch", + "mode": "auto", + "program": "${workspaceFolder}", + "args": [ + "--version" + ], + "console": "integratedTerminal" + }, + { + "name": "Debug with custom args", + "type": "go", + "request": "launch", + "mode": "auto", + "program": "${workspaceFolder}", + "args": [], + "console": "integratedTerminal" + }, + { + "name": "Test Current Package", + "type": "go", + "request": "launch", + "mode": "test", + "program": "${fileDirname}", + "console": "integratedTerminal" + }, + { + "name": "Test Current File", + "type": "go", + "request": "launch", + "mode": "test", + "program": "${fileDirname}", + "args": [ + "-test.run", + "${input:testName}" + ], + "console": "integratedTerminal" + } + ], + "inputs": [ + { + "id": "testName", + "description": "Enter test function name pattern", + "default": "TestExample", + "type": "promptString" + } + ] +} \ No newline at end of file diff --git a/.vscode/settings.json b/.vscode/settings.json new file mode 100644 index 00000000..4cfc4ea9 --- /dev/null +++ b/.vscode/settings.json @@ -0,0 +1,120 @@ +{ + // Go-specific settings + "go.lintTool": "golangci-lint", + "go.generateTestsFlags": [ + "-parallel" + ], + "gopls": { + "build.buildFlags": [ + // uncomment to build compile beta functionality in vscode + //"-tags=beta" + ] + }, + // File associations + "files.associations": { + "*.proto": "proto3", + "Makefile": "makefile", + "*.mk": "makefile", + "*.tmpl": "go-template" + }, + // Exclude common Go build artifacts and vendor directories + "files.exclude": { + "**/vendor": true, + "**/pingcli": true, + "**/*.exe": true + }, + // Search exclusions + "search.exclude": { + "**/vendor": true, + }, + // Markdown settings for documentation + "markdown.preview.breaks": true, + "markdown.preview.linkify": true, + // YAML settings for configuration files + "yaml.format.enable": true, + "yaml.validate": true, + // JSON settings + "json.format.enable": true, + // Git settings + "git.ignoreLimitWarning": true, + // Spell checker settings + "cSpell.words": [ + "authn", + "authz", + "cobra", + "davinci", + "devcheck", + "devchecknotest", + "Getenv", + "godoc", + "goimports", + "golang", + "golangci", + "golangcilint", + "gomod", + "GOPATH", + "gosec", + "grpc", + "hclog", + "importfmtlint", + "InitKoanfs", + "koanf", + "mfas", + "oauth", + "oidc", + "patrickcping", + "pflag", + "pingcli", + "pingfederate", + "pingidentity", + "pingone", + "protobuf", + "removetestcontainer", + "saml", + "scim", + "spincontainer", + "testutils", + "zerolog" + ], + "cSpell.ignoreWords": [ + "bool", + "func", + "int64", + "string", + "struct", + "testdata", + "uint", + ], + "cSpell.languageSettings": [ + { + "languageId": "go", + "caseSensitive": true + } + ], + // Language-specific settings + "[go]": { + "editor.tabSize": 4, + "editor.insertSpaces": false, + "editor.formatOnSave": true, + }, + "[markdown]": { + "editor.wordWrap": "on", + "editor.quickSuggestions": { + "comments": "off", + "strings": "off", + "other": "off" + } + }, + "[yaml]": { + "editor.tabSize": 2, + "editor.insertSpaces": true + }, + "[json]": { + "editor.tabSize": 2, + "editor.insertSpaces": true + }, + "[jsonc]": { + "editor.tabSize": 2, + "editor.insertSpaces": true + } +} \ No newline at end of file diff --git a/.vscode/tasks.json b/.vscode/tasks.json new file mode 100644 index 00000000..97c8f1fe --- /dev/null +++ b/.vscode/tasks.json @@ -0,0 +1,219 @@ +{ + "version": "2.0.0", + "tasks": [ + { + "label": "install", + "type": "shell", + "command": "make", + "args": [ + "install" + ], + "group": { + "kind": "build", + "isDefault": true + }, + "presentation": { + "echo": true, + "reveal": "always", + "focus": false, + "panel": "shared", + "showReuseMessage": true, + "clear": false + }, + "problemMatcher": "$go" + }, + { + "label": "test", + "type": "shell", + "command": "make", + "args": [ + "test" + ], + "group": "test", + "presentation": { + "echo": true, + "reveal": "always", + "focus": false, + "panel": "shared", + "showReuseMessage": true, + "clear": false + }, + "problemMatcher": "$go" + }, + { + "label": "fmt", + "type": "shell", + "command": "make", + "args": [ + "fmt" + ], + "group": "build", + "presentation": { + "echo": true, + "reveal": "silent", + "focus": false, + "panel": "shared", + "showReuseMessage": true, + "clear": false + }, + "problemMatcher": "$go" + }, + { + "label": "vet", + "type": "shell", + "command": "make", + "args": [ + "vet" + ], + "group": "build", + "presentation": { + "echo": true, + "reveal": "always", + "focus": false, + "panel": "shared", + "showReuseMessage": true, + "clear": false + }, + "problemMatcher": "$go" + }, + { + "label": "golangcilint", + "type": "shell", + "command": "make", + "args": [ + "golangcilint" + ], + "group": "build", + "presentation": { + "echo": true, + "reveal": "always", + "focus": false, + "panel": "shared", + "showReuseMessage": true, + "clear": false + }, + "problemMatcher": "$go" + }, + { + "label": "devchecknotest", + "type": "shell", + "command": "make", + "args": [ + "devchecknotest" + ], + "group": "build", + "presentation": { + "echo": true, + "reveal": "always", + "focus": false, + "panel": "shared", + "showReuseMessage": true, + "clear": false + }, + "problemMatcher": "$go" + }, + { + "label": "devcheck", + "type": "shell", + "command": "make", + "args": [ + "devcheck" + ], + "group": "test", + "presentation": { + "echo": true, + "reveal": "always", + "focus": false, + "panel": "shared", + "showReuseMessage": true, + "clear": false + }, + "problemMatcher": "$go" + }, + { + "label": "spincontainer", + "type": "shell", + "command": "make", + "args": [ + "spincontainer" + ], + "group": "test", + "presentation": { + "echo": true, + "reveal": "always", + "focus": false, + "panel": "shared", + "showReuseMessage": true, + "clear": false + }, + "isBackground": true, + "problemMatcher": [] + }, + { + "label": "removetestcontainer", + "type": "shell", + "command": "make", + "args": [ + "removetestcontainer" + ], + "group": "test", + "presentation": { + "echo": true, + "reveal": "always", + "focus": false, + "panel": "shared", + "showReuseMessage": true, + "clear": false + }, + "problemMatcher": [] + }, + { + "label": "build local", + "type": "shell", + "command": "go", + "args": [ + "build", + "-o", + "pingcli", + "." + ], + "group": "build", + "presentation": { + "echo": true, + "reveal": "silent", + "focus": false, + "panel": "shared", + "showReuseMessage": true, + "clear": false + }, + "problemMatcher": "$go" + }, + { + "label": "run with args", + "type": "shell", + "command": "./pingcli", + "args": [ + "${input:cliArgs}" + ], + "group": "build", + "dependsOn": "build local", + "presentation": { + "echo": true, + "reveal": "always", + "focus": false, + "panel": "shared", + "showReuseMessage": true, + "clear": false + }, + "problemMatcher": [] + } + ], + "inputs": [ + { + "id": "cliArgs", + "description": "Enter CLI arguments", + "default": "--help", + "type": "promptString" + } + ] +} \ No newline at end of file diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 00000000..87ceead1 --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,22 @@ +# Contributing to the Ping CLI + +We appreciate your help! We welcome contributions in the form of creating issues or pull requests. + +Know that: + +1. If you have any questions, please ask! We'll help as best we can +2. While we appreciate perfect PRs, it's not essential. We'll fix up any housekeeping changes before merge. Any PRs that need further work, we'll point you in the right direction or can take on ourselves. +3. We may not be able to respond quickly, our development cycles are on a priority basis. +4. We base our priorities on customer need and the number of votes on issues/PRs by the number of 👍 reactions. If there is an existing issue or PR for something you'd like, please vote! +5. Some files are created and maintained by an internal generator project. These files are marked with comments at the top of the file. We may not be able to merge changes to these files as changes will be overwritten by the generator. + +The following guides are there to help you get started with development, logging issues and creating PRs. Following the guides allows us to respond quicker and will result in faster PR merges. + +## Getting Started + +- [Set Up Your Development Environment](contributing/development-environment.md) +- [Project Design](contributing/project-design.md) +- [Acceptance Test Strategy](contributing/acceptance-test-strategy.md) + +## Process +- [Raise Pull Request Checklist](contributing/pr-checklist.md) diff --git a/contributing/acceptance-test-strategy.md b/contributing/acceptance-test-strategy.md new file mode 100644 index 00000000..14a19038 --- /dev/null +++ b/contributing/acceptance-test-strategy.md @@ -0,0 +1,988 @@ +# Acceptance Testing Strategy + +This document outlines the comprehensive testing strategy for the PingCLI, based on analysis of existing test patterns and CLI testing best practices. All new commands, connectors, and functionality should follow these testing patterns to ensure consistency, reliability, and maintainability. + +## Overview + +The CLI uses Go's built-in testing framework with acceptance tests that interact with real Ping Identity environments (PingOne and PingFederate). Tests are organized by command/functionality area and follow consistent naming and structure patterns. The testing strategy encompasses unit tests for command logic, integration tests for API interactions, and end-to-end tests for complete CLI workflows. + +## Test Organization + +### File Structure +- **Command tests**: `_test.go` files co-located with command implementations +- **Connector/Integration tests**: Located in `internal/connector//` directories +- **Test utilities**: Located in `internal/testing/` with specialized helpers: + - `testutils/` - General test utilities and API client setup + - `testutils_cobra/` - Cobra command testing helpers + - `testutils_koanf/` - Configuration testing helpers + - `testutils_resource/` - Resource-specific test data generators + - `testutils_terraform/` - Terraform plan validation helpers + +### Package Structure +- Tests are placed in `_test` packages (e.g., `platform_test`, `config_test`) +- Import test helpers from `internal/testing/testutils*` packages +- Use shared client configurations from `testutils.GetClientInfo()` +- Container-based testing for PingFederate integration via Docker + +## Core Testing Patterns + +### 1. Standard CLI Command Tests + +Every CLI command should implement these core test functions: + +#### **Basic Execution Tests** +Tests that commands execute successfully with valid parameters: +```go +func TestCmd_Execute(t *testing.T) { + // Test command executes without error with minimal valid parameters + // Use testutils_cobra.ExecutePingcli() for command execution + // Use testutils.CheckExpectedError() to validate no error occurred +} +``` + +#### **Argument Validation Tests** +Tests command argument validation and error handling: +```go +func TestCmd_TooManyArgs(t *testing.T) { + // Test command rejects invalid argument counts + expectedErrorPattern := `^failed to execute 'pingcli ': command accepts X arg\(s\), received Y$` + err := testutils_cobra.ExecutePingcli(t, "", "extra-arg") + testutils.CheckExpectedError(t, err, &expectedErrorPattern) +} + +func TestCmd_InvalidFlag(t *testing.T) { + // Test command rejects invalid flags + expectedErrorPattern := `^unknown flag: --invalid$` + err := testutils_cobra.ExecutePingcli(t, "", "--invalid") + testutils.CheckExpectedError(t, err, &expectedErrorPattern) +} +``` + +#### **Flag Validation Tests** +Tests all supported flags with valid and invalid values: +```go +func TestCmd_Flag(t *testing.T) { + // Test each flag with valid values + // Test flag combinations and interdependencies + // Test required flag groups (marked with cobra's MarkFlagsRequiredTogether) +} + +func TestCmd_FlagInvalid(t *testing.T) { + // Test flags with invalid values + // Validate appropriate error messages +} +``` + +### 2. Integration and API Testing Strategy + +#### **Authentication Testing** +- Test all supported authentication methods for each service +- Validate proper error handling for invalid credentials +- Test credential flag combinations and requirements + +```go +func TestCmd_PingOneWorkerFlags(t *testing.T) { + // Test PingOne worker authentication with valid credentials + // Use environment variables for actual credentials + err := testutils_cobra.ExecutePingcli(t, "", + "--pingone-worker-environment-id", os.Getenv("TEST_PINGONE_ENVIRONMENT_ID"), + "--pingone-worker-client-id", os.Getenv("TEST_PINGONE_WORKER_CLIENT_ID"), + "--pingone-worker-client-secret", os.Getenv("TEST_PINGONE_WORKER_CLIENT_SECRET"), + "--pingone-region-code", os.Getenv("TEST_PINGONE_REGION_CODE")) + testutils.CheckExpectedError(t, err, nil) +} + +func TestCmd_PingFederateBasicAuthFlags(t *testing.T) { + // Test PingFederate basic authentication + // Use container-based testing for PingFederate +} +``` + +#### **Export/Import Testing** +For commands that export or import configurations: +- Test minimal exports (specific services/resources) +- Test comprehensive exports (all available data) +- Test different output formats (HCL, Terraform) +- Validate exported data completeness and accuracy + +```go +func TestCmd_ExportFormats(t *testing.T) { + // Test each supported export format + // Validate output file structure and content + // Test format-specific features and limitations +} +``` + +#### **Container Integration Testing** +For PingFederate-related functionality: +- Use Docker containers for consistent test environments +- Test against known container configurations +- Validate SSL/TLS certificate handling + +```go +// Container tests should be marked for container execution +func TestCmd_ContainerIntegration(t *testing.T) { + // This test requires the PingFederate container to be running + // Use testutils.GetClientInfo() to get configured PingFederate client +} +``` + +### 3. Test Environment Setup and Validation + +The CLI testing framework uses environment variables and helper functions to validate test requirements before execution. + +#### **Essential Environment Variables** + +**PingOne Integration Testing** +Required for all PingOne-related tests: +- `TEST_PINGONE_ENVIRONMENT_ID` - PingOne environment for testing +- `TEST_PINGONE_REGION_CODE` - PingOne region (e.g., 'NA', 'EU', 'AP') +- `TEST_PINGONE_WORKER_CLIENT_ID` - Worker application client ID with admin roles +- `TEST_PINGONE_WORKER_CLIENT_SECRET` - Worker application client secret + +**PingFederate Container Testing** +Required for PingFederate integration tests: +- `TEST_PING_IDENTITY_DEVOPS_USER` - DevOps program username +- `TEST_PING_IDENTITY_DEVOPS_KEY` - DevOps program access key +- `TEST_PING_IDENTITY_ACCEPT_EULA` - Must be set to "YES" to accept EULA + +#### **Test Setup Patterns** + +**Standard CLI Test Setup** +```go +func TestCmd_(t *testing.T) { + // Initialize configuration system for testing + testutils_koanf.InitKoanfs(t) + + // Create temporary directories as needed + outputDir := t.TempDir() + + // Execute CLI command with test parameters + err := testutils_cobra.ExecutePingcli(t, "command", "subcommand", "--flag", "value") + testutils.CheckExpectedError(t, err, nil) +} +``` + +**Integration Test Setup** +```go +func TestCmd_Integration(t *testing.T) { + // Get configured API clients + clientInfo := testutils.GetClientInfo(t) + + // Use actual API clients for integration testing + // clientInfo.PingOneApiClient, clientInfo.PingFederateApiClient +} +``` + +#### **Container Environment Management** + +**PingFederate Container Lifecycle** +The CLI includes Make targets for managing PingFederate containers: + +```bash +# Start PingFederate container for testing +make spincontainer + +# Run tests requiring PingFederate +make test + +# Clean up container after testing +make removetestcontainer +``` + +**Container Test Requirements** +- Docker must be running and accessible +- DevOps program credentials must be configured +- Container health checks must pass before tests execute +- Tests should clean up any configuration changes made during testing + +**Container Test Patterns** +```go +func TestPingFederateIntegration(t *testing.T) { + // Container tests should validate container availability + clientInfo := testutils.GetClientInfo(t) + if clientInfo.PingFederateApiClient == nil { + t.Skip("PingFederate container not available") + } + + // Use configured client for testing + // Test should be idempotent and not affect other tests +} +``` + +#### **CLI-Specific Test Considerations** + +**Configuration Management** +- Use `testutils_koanf.InitKoanfs(t)` to initialize the configuration system +- Test both command-line flags and configuration file scenarios +- Validate configuration precedence (flags override config files) + +**Output Format Testing** +- Test different output formats (JSON, table, quiet modes) +- Validate machine-readable output for automation scenarios +- Test human-readable output formatting + +**Error Message Testing** +- Validate specific error message patterns using regex +- Test error scenarios for invalid inputs, network issues, authentication failures +- Ensure error messages provide actionable guidance to users + +### 4. Export and Data Validation Testing + +#### **Export Functionality Testing** +For platform export and similar data extraction commands: + +**Export Format Validation** +```go +func TestCmd_ExportFormat(t *testing.T) { + // Test each supported export format (HCL, Terraform, etc.) + // Validate format-specific output structure + // Test format conversion accuracy +} +``` + +**Export Completeness Testing** +```go +func TestExportCompleteness(t *testing.T) { + // Use testutils.ValidateImportBlocks() to verify exported data + // Test that all expected resources are exported + // Validate resource relationships and dependencies +} +``` + +**Service-Specific Export Testing** +```go +func TestCmd_Service(t *testing.T) { + // Test export of specific services (PingOne SSO, MFA, Protect, etc.) + // Validate service-specific resource types and configurations + // Test service filtering and selection +} +``` + +### 5. Configuration and Profile Testing + +#### **Profile Management Testing** +For config-related commands that manage profiles and settings: + +**Profile CRUD Operations** +```go +func TestConfigProfile_(t *testing.T) { + // Test profile creation, reading, updating, deletion + // Validate profile persistence and retrieval + // Test profile switching and activation +} +``` + +**Configuration Key Management** +```go +func TestConfigKey_(t *testing.T) { + // Test setting and getting configuration values + // Validate configuration validation and type checking + // Test configuration inheritance and precedence +} +``` + +**Authentication Flow Testing** +```go +func TestAuth_(t *testing.T) { + // Test login/logout flows + // Validate token persistence and refresh + // Test authentication error handling +} +``` + +### 6. Request and Custom API Testing + +#### **Custom Request Testing** +For request command functionality that makes custom API calls: + +**HTTP Method Testing** +```go +func TestRequest_(t *testing.T) { + // Test GET, POST, PUT, DELETE, PATCH operations + // Validate request formatting and response handling + // Test authentication integration with custom requests +} +``` + +**API Compatibility Testing** +```go +func TestRequest_APICompatibility(t *testing.T) { + // Test against known API endpoints + // Validate response parsing and error handling + // Test API versioning and compatibility matrices +} +``` + +#### **Plugin System Testing** +For plugin management functionality: + +**Plugin Lifecycle Testing** +```go +func TestPlugin_(t *testing.T) { + // Test plugin add, list, remove operations + // Validate plugin discovery and loading + // Test plugin execution and integration +} +``` + +## Advanced Testing Patterns + +### 1. CLI-Specific Testing Considerations + +#### **Command Chaining and Workflow Testing** +Test complex workflows that involve multiple CLI commands: +```go +func TestWorkflow_(t *testing.T) { + // Test sequences like: login -> export -> configure -> logout + // Validate state persistence between commands + // Test failure recovery and cleanup +} +``` + +#### **Interactive vs Non-Interactive Testing** +```go +func TestInteractive_(t *testing.T) { + // Test commands with interactive prompts + // Use testutils.WriteStringToPipe() for input simulation + // Validate prompt text and response handling +} +``` + +#### **Cross-Platform Compatibility** +- Test path handling differences (Windows vs Unix) +- Validate file permission handling +- Test shell integration and autocompletion + +### 2. Performance and Scale Testing + +#### **Large Data Set Testing** +```go +func Test_LargeDataSet(t *testing.T) { + // Test commands with large numbers of resources + // Validate memory usage and processing time + // Test pagination and batching logic +} +``` + +#### **Concurrent Execution Testing** +```go +func Test_Concurrent(t *testing.T) { + // Test multiple CLI instances running simultaneously + // Validate file locking and state management + // Test API rate limiting and retry logic +} +``` + +### 3. Error Handling and Recovery Testing + +#### **Network Error Simulation** +```go +func Test_NetworkErrors(t *testing.T) { + // Test behavior with network timeouts + // Test API unavailability scenarios + // Validate retry logic and exponential backoff +} +``` + +#### **Authentication Error Testing** +```go +func Test_AuthenticationErrors(t *testing.T) { + // Test expired tokens and credentials + // Test insufficient permissions + // Validate error message clarity and actionability +} +``` + +#### **File System Error Testing** +```go +func Test_FileSystemErrors(t *testing.T) { + // Test disk space issues + // Test permission denied scenarios + // Test file corruption and recovery +} +``` + +### 4. Security and Credential Testing + +#### **Credential Storage Testing** +```go +func TestCredentialStorage_(t *testing.T) { + // Test secure storage of tokens and credentials + // Validate encryption and access controls + // Test credential cleanup and expiration +} +``` + +#### **Sensitive Data Handling** +```go +func TestSensitiveData_(t *testing.T) { + // Test that sensitive data is not logged or exposed + // Validate redaction in error messages and output + // Test secure temporary file handling +} +``` + +### 5. CLI Feature Deprecation and Backward Compatibility + +When deprecating CLI flags, commands, or changing behavior: + +#### **Deprecation Strategy** +- **Simultaneous Support**: Both deprecated and new functionality must work during the deprecation period +- **Gradual Migration**: Users should be able to migrate incrementally without breaking changes +- **Clear Warnings**: Deprecated flags/commands should generate helpful deprecation warnings +- **Documentation**: Update both help text and user documentation + +#### **Required Deprecation Tests** +```go +func Test_Deprecation_(t *testing.T) { + // Test deprecated flag works (legacy usage) + // Test new flag works (current usage) + // Test both flags work together (migration period) + // Test migration path from deprecated to new flag + // Validate deprecation warnings are generated +} +``` + +#### **Backward Compatibility Validation** +- Existing user scripts and workflows must continue to work unchanged +- No functional regression during deprecation period +- Smooth migration path from old to new CLI interfaces +- Proper handling of edge cases during transition + +### 6. Multi-Service Integration Testing + +For commands that work across multiple Ping Identity services: +- Test service interoperability and data consistency +- Test partial service availability scenarios +- Validate cross-service authentication and authorization +- Test service-specific configuration and feature differences + +### 7. Configuration Migration and Upgrade Testing + +#### **Configuration File Format Regression Testing** +When modifying configuration file formats, schema, or default values: + +**Legacy Configuration Compatibility** +```go +func TestConfig_LegacyFormatCompatibility(t *testing.T) { + // Test that existing configuration files continue to work + // Load pre-upgrade configuration samples + // Validate that all settings are properly migrated + // Test that no user data is lost during upgrade +} +``` + +**Configuration Schema Evolution** +```go +func TestConfig_SchemaEvolution_v_to_v(t *testing.T) { + // Test specific schema version transitions + // Validate field renames, type changes, and removals + // Test default value handling for new fields + // Verify backward compatibility warnings are shown +} +``` + +**Configuration File Migration** +```go +func TestConfig_Migration_(t *testing.T) { + // Create legacy configuration file + legacyConfig := ` + { + "old_field": "value", + "deprecated_setting": true + }` + + configFile := testutils.WriteStringToPipe(t, legacyConfig) + defer configFile.Close() + + // Test that CLI can load and migrate the configuration + err := testutils_cobra.ExecutePingcli(t, "config", "migrate", "--config-file", configFile.Name()) + testutils.CheckExpectedError(t, err, nil) + + // Validate migrated configuration + // Check that new format is used + // Verify all data was preserved +} +``` + +## Configuration Regression Testing Strategy + +### Configuration File Format Testing + +#### **Version Compatibility Matrix Testing** +Create comprehensive tests for configuration file compatibility across versions: + +```go +func TestConfigCompatibility_Matrix(t *testing.T) { + testCases := []struct { + version string + configFile string + expectError bool + description string + }{ + { + version: "v1.0.0", + configFile: "testdata/configs/v1.0.0-sample.json", + expectError: false, + description: "Legacy v1.0.0 format should still work", + }, + { + version: "v1.5.0", + configFile: "testdata/configs/v1.5.0-with-new-fields.yaml", + expectError: false, + description: "v1.5.0 format with new fields", + }, + { + version: "v2.0.0", + configFile: "testdata/configs/v2.0.0-breaking-changes.yaml", + expectError: false, + description: "v2.0.0 format after breaking changes", + }, + } + + for _, tc := range testCases { + t.Run(tc.description, func(t *testing.T) { + err := testutils_cobra.ExecutePingcli(t, "config", "validate", "--config-file", tc.configFile) + if tc.expectError { + testutils.CheckExpectedError(t, err, &expectedErrorPattern) + } else { + testutils.CheckExpectedError(t, err, nil) + } + }) + } +} +``` + +#### **Configuration Migration Testing** +Test automatic migration of configuration files during upgrades: + +```go +func TestConfigMigration_AutoUpgrade(t *testing.T) { + // Create a temporary directory for config testing + configDir := t.TempDir() + + // Create legacy configuration file + legacyConfigPath := filepath.Join(configDir, "config.yaml") + legacyConfig := ` +version: "1.0" +profiles: + default: + pingone: + environment_id: "old-format-env-id" + client_credentials: + client_id: "test-client" + client_secret: "test-secret" + region: "NA"` + + err := os.WriteFile(legacyConfigPath, []byte(legacyConfig), 0644) + require.NoError(t, err) + + // Test that CLI automatically migrates the config on first use + err = testutils_cobra.ExecutePingcli(t, "config", "get", + "--config-file", legacyConfigPath, + "profiles.default.pingone.environment_id") + testutils.CheckExpectedError(t, err, nil) + + // Read the config file back and verify it was migrated + migratedConfig, err := os.ReadFile(legacyConfigPath) + require.NoError(t, err) + + // Verify new format is used + assert.Contains(t, string(migratedConfig), `version: "2.0"`) + assert.Contains(t, string(migratedConfig), `worker_environment_id:`) // New field name +} +``` + +#### **Configuration Backup and Recovery Testing** +```go +func TestConfigUpgrade_BackupAndRecovery(t *testing.T) { + configDir := t.TempDir() + originalConfigPath := filepath.Join(configDir, "config.yaml") + backupConfigPath := filepath.Join(configDir, "config.yaml.backup") + + // Create original configuration + originalConfig := generateLegacyConfig("1.5.0") + err := os.WriteFile(originalConfigPath, []byte(originalConfig), 0644) + require.NoError(t, err) + + // Test that upgrade creates backup + err = testutils_cobra.ExecutePingcli(t, "config", "upgrade", + "--config-file", originalConfigPath, + "--create-backup") + testutils.CheckExpectedError(t, err, nil) + + // Verify backup was created + assert.FileExists(t, backupConfigPath) + + // Verify backup contains original content + backupContent, err := os.ReadFile(backupConfigPath) + require.NoError(t, err) + assert.Equal(t, originalConfig, string(backupContent)) + + // Test recovery from backup + err = testutils_cobra.ExecutePingcli(t, "config", "restore", + "--backup-file", backupConfigPath, + "--config-file", originalConfigPath) + testutils.CheckExpectedError(t, err, nil) +} +``` + +### Configuration Schema Evolution Testing + +#### **Field Rename Testing** +```go +func TestConfigSchema_FieldRenames(t *testing.T) { + // Test configuration files with renamed fields + renamedFields := map[string]string{ + "environment_id": "worker_environment_id", + "client_id": "worker_client_id", + "client_secret": "worker_client_secret", + "region": "region_code", + } + + for oldField, newField := range renamedFields { + t.Run(fmt.Sprintf("%s_to_%s", oldField, newField), func(t *testing.T) { + // Create config with old field name + legacyConfig := fmt.Sprintf(` +profiles: + test: + pingone: + %s: "test-value"`, oldField) + + configFile := testutils.WriteStringToPipe(t, legacyConfig) + defer configFile.Close() + + // Test that old field name still works (with deprecation warning) + err := testutils_cobra.ExecutePingcli(t, "config", "validate", "--config-file", configFile.Name()) + testutils.CheckExpectedError(t, err, nil) + + // Test that new field name works + newConfig := fmt.Sprintf(` +profiles: + test: + pingone: + %s: "test-value"`, newField) + + newConfigFile := testutils.WriteStringToPipe(t, newConfig) + defer newConfigFile.Close() + + err = testutils_cobra.ExecutePingcli(t, "config", "validate", "--config-file", newConfigFile.Name()) + testutils.CheckExpectedError(t, err, nil) + }) + } +} +``` + +#### **Default Value Evolution Testing** +```go +func TestConfigSchema_DefaultValueEvolution(t *testing.T) { + testCases := []struct { + configVersion string + expectedDefaults map[string]interface{} + description string + }{ + { + configVersion: "1.0", + expectedDefaults: map[string]interface{}{ + "output_format": "table", + "color_output": true, + "region_code": "NA", + }, + description: "v1.0 defaults", + }, + { + configVersion: "2.0", + expectedDefaults: map[string]interface{}{ + "output_format": "json", // Changed default + "color_output": "auto", // Changed from bool to string + "region_code": "NA", + "verify_tls": true, // New field with default + }, + description: "v2.0 defaults with new fields", + }, + } + + for _, tc := range testCases { + t.Run(tc.description, func(t *testing.T) { + // Create minimal config for version + minimalConfig := fmt.Sprintf(`version: "%s"`, tc.configVersion) + configFile := testutils.WriteStringToPipe(t, minimalConfig) + defer configFile.Close() + + // Test that defaults are applied correctly + for key, expectedValue := range tc.expectedDefaults { + output, err := testutils_cobra.ExecutePingcliWithOutput(t, "config", "get", + "--config-file", configFile.Name(), key) + testutils.CheckExpectedError(t, err, nil) + assert.Contains(t, output, fmt.Sprintf("%v", expectedValue)) + } + }) + } +} +``` + +### Configuration Test Data Management + +#### **Test Configuration File Generators** +```go +// In internal/testing/testutils_config/ +func GenerateLegacyConfig(version string, profiles map[string]interface{}) string { + // Generate configuration files in legacy formats + // Support different versions and profile structures + config := map[string]interface{}{ + "version": version, + "profiles": profiles, + } + + yamlData, _ := yaml.Marshal(config) + return string(yamlData) +} + +func GenerateCurrentConfig(profiles map[string]interface{}) string { + // Generate configuration files in current format + return GenerateLegacyConfig("2.0", profiles) +} + +func CreateTestProfile(envID, clientID, clientSecret, region string) map[string]interface{} { + // Create standardized test profile structure + return map[string]interface{}{ + "pingone": map[string]interface{}{ + "worker_environment_id": envID, + "worker_client_id": clientID, + "worker_client_secret": clientSecret, + "region_code": region, + }, + } +} +``` + +#### **Configuration Validation Helpers** +```go +func ValidateConfigMigration(t *testing.T, beforePath, afterPath string) { + // Helper to validate that configuration migration preserved all data + beforeConfig := loadConfigFile(t, beforePath) + afterConfig := loadConfigFile(t, afterPath) + + // Check that no functional settings were lost + validateProfilesEquivalent(t, beforeConfig.Profiles, afterConfig.Profiles) + + // Verify new format compliance + assert.True(t, isValidCurrentFormat(afterConfig)) + + // Validate that functionality still works + validateConfigFunctionality(t, afterPath) +} + +func CompareConfigValues(t *testing.T, config1, config2 interface{}, ignoredFields []string) { + // Deep compare configuration values + // Ignore version metadata and timestamps + // Validate functional equivalence + + normalized1 := normalizeConfig(config1, ignoredFields) + normalized2 := normalizeConfig(config2, ignoredFields) + + assert.Equal(t, normalized1, normalized2, "Configuration values should be functionally equivalent") +} +``` + +## Test Configuration Patterns + +### CLI Command Test Helpers + +Use consistent command testing patterns: +```go +func testCmd_(t *testing.T) { + // Initialize configuration system + testutils_koanf.InitKoanfs(t) + + // Set up test data/environment as needed + outputDir := t.TempDir() + + // Execute command with specific parameters + err := testutils_cobra.ExecutePingcli(t, "command", "subcommand", + "--flag1", "value1", + "--flag2", "value2") + + // Validate results + testutils.CheckExpectedError(t, err, expectedErrorPattern) +} +``` + +### Test Data Generation + +Implement test data generators in `internal/testing/testutils_resource/`: +```go +func GenerateTestData() { + // Generate consistent test data for resources + // Use deterministic values for repeatability + // Include edge cases and boundary conditions +} +``` + +### Integration Test Helpers + +Implement API integration helpers: +```go +func setupTestEnvironment(t *testing.T) *connector.ClientInfo { + clientInfo := testutils.GetClientInfo(t) + + // Validate required services are available + if clientInfo.PingOneApiClient == nil { + t.Skip("PingOne API client not configured") + } + + return clientInfo +} +``` + +## Quality Standards + +### Test Coverage Requirements + +All CLI commands and major functionality must have: +- [ ] **Basic execution testing** with valid parameters +- [ ] **Argument validation testing** (too many/few arguments) +- [ ] **Flag validation testing** with valid and invalid values +- [ ] **Required flag group testing** (flags that must be used together) +- [ ] **Help flag testing** (--help, -h) +- [ ] **Error condition testing** with expected error patterns +- [ ] **Authentication testing** for all supported auth methods +- [ ] **Integration testing** with real API endpoints (where applicable) + +#### **Additional Requirements for CLI Changes** +When modifying existing commands: +- [ ] **Deprecation testing** for any removed or changed flags/commands +- [ ] **Migration path validation** from old to new CLI interfaces +- [ ] **Dual functionality testing** during deprecation periods +- [ ] **Warning validation** for deprecated flag/command usage +- [ ] **Backward compatibility testing** for existing user workflows + +#### **Configuration Migration Requirements** +When changing configuration file formats or schema: +- [ ] **Version compatibility matrix testing** across supported versions +- [ ] **Automatic migration testing** for seamless upgrades +- [ ] **Backup and recovery testing** to prevent data loss +- [ ] **Field rename/restructure testing** with proper deprecation warnings +- [ ] **Default value evolution testing** to ensure consistency +- [ ] **Configuration validation testing** for all supported formats + +#### **Export/Import Command Requirements** +For data export/import commands: +- [ ] **Format validation testing** for all supported output formats +- [ ] **Completeness testing** using `testutils.ValidateImportBlocks()` +- [ ] **Service filtering testing** (specific services vs. all services) +- [ ] **Overwrite behavior testing** (existing vs. new output directories) + +### Test Reliability + +- Use `t.Parallel()` for tests that can run concurrently +- Implement proper cleanup for temporary files and directories +- Use `t.TempDir()` for temporary directory creation +- Handle container lifecycle properly for PingFederate tests +- Validate test stability across multiple runs +- Use deterministic test data and avoid random values + +### Test Organization and Maintenance + +- Follow naming convention: `TestCmd_` +- Keep test functions focused on single scenarios +- Use descriptive test and variable names +- Group related tests in the same file as the command implementation +- Document complex test scenarios and setup requirements +- Regular review and update of test patterns as CLI evolves +- Ensure container tests work in CI/CD environments + +## Examples + +### Complete CLI Command Test Structure +```go +// Basic execution test +func TestCmd_Execute(t *testing.T) { + testutils_koanf.InitKoanfs(t) + outputDir := t.TempDir() + + err := testutils_cobra.ExecutePingcli(t, "command", "subcommand", + "--output-directory", outputDir, + "--overwrite") + testutils.CheckExpectedError(t, err, nil) +} + +// Argument validation tests +func TestCmd_TooManyArgs(t *testing.T) { /* ... */ } +func TestCmd_InvalidFlag(t *testing.T) { /* ... */ } + +// Flag testing +func TestCmd_Flag(t *testing.T) { /* ... */ } +func TestCmd_FlagInvalid(t *testing.T) { /* ... */ } + +// Integration tests +func TestCmd_PingOneIntegration(t *testing.T) { /* ... */ } +func TestCmd_PingFederateContainerIntegration(t *testing.T) { /* ... */ } + +// For commands with deprecated functionality +func TestCmd_Deprecation_(t *testing.T) { + // Legacy flag usage test + // New flag usage test + // Migration path test + // Warning validation test +} +``` + +### Complete Export Command Test Structure +```go +func TestPlatformExportCmd_Execute(t *testing.T) { /* Basic execution */ } +func TestPlatformExportCmd_ServiceGroupFlag(t *testing.T) { /* Service filtering */ } +func TestPlatformExportCmd_ExportFormatFlag(t *testing.T) { /* Format testing */ } +func TestPlatformExportCmd_PingOneWorkerFlags(t *testing.T) { /* Authentication */ } +func TestPlatformExportCmd_ContainerIntegration(t *testing.T) { /* Container tests */ } +``` + +### Complete Integration Test Structure +```go +func TestConnector__Export(t *testing.T) { + clientInfo := testutils.GetClientInfo(t) + resource := &connector.Resource{ClientInfo: clientInfo} + + // Test export functionality + testutils.ValidateImportBlocks(t, resource, expectedBlocks) +} +``` + +### Complete Configuration Migration Test Structure +```go +func TestConfigMigration_v_to_v(t *testing.T) { + // Setup legacy configuration + legacyConfig := testutils_config.GenerateLegacyConfig("1.0", testProfiles) + configPath := filepath.Join(t.TempDir(), "config.yaml") + + // Test migration process + err := testutils_cobra.ExecutePingcli(t, "config", "upgrade", "--config-file", configPath) + testutils.CheckExpectedError(t, err, nil) + + // Validate migration results + testutils_config.ValidateConfigMigration(t, legacyConfigPath, configPath) +} + +func TestConfigCompatibility_AllVersions(t *testing.T) { + versions := []string{"1.0.0", "1.5.0", "2.0.0", "2.1.0"} + + for _, version := range versions { + t.Run(fmt.Sprintf("version_%s", version), func(t *testing.T) { + configFile := fmt.Sprintf("testdata/configs/sample-%s.yaml", version) + + // Test that configuration loads without error + err := testutils_cobra.ExecutePingcli(t, "config", "validate", "--config-file", configFile) + testutils.CheckExpectedError(t, err, nil) + + // Test that functionality works with this configuration + err = testutils_cobra.ExecutePingcli(t, "platform", "export", + "--config-file", configFile, + "--dry-run") + testutils.CheckExpectedError(t, err, nil) + }) + } +} +``` + +This comprehensive testing strategy ensures that all CLI functionality is thoroughly validated, providing confidence in the reliability and correctness of the PingCLI across different environments and use cases. The configuration migration testing ensures users can seamlessly upgrade between CLI versions without losing their settings or experiencing breaking changes. \ No newline at end of file diff --git a/contributing/development-environment.md b/contributing/development-environment.md new file mode 100644 index 00000000..074789da --- /dev/null +++ b/contributing/development-environment.md @@ -0,0 +1,185 @@ +# Development Environment Setup + +## Requirements + +- [Go](https://golang.org/doc/install) 1.25.1+ (to build and test the CLI) +- [Docker](https://docs.docker.com/get-docker/) (to run PingFederate integration tests) +- [Git](https://git-scm.com/downloads) (for version control) +- Access to a PingOne environment (for integration testing) + +## Quick Start + +If you wish to work on the CLI, you'll first need [Go](http://www.golang.org) installed on your machine (please check the [requirements](#requirements) before proceeding). + +*Note:* This project uses [Go Modules](https://blog.golang.org/using-go-modules) making it safe to work with it outside of your existing [GOPATH](http://golang.org/doc/code.html#GOPATH). The instructions that follow assume a directory in your home directory outside of the standard GOPATH (i.e `$HOME/development/pingidentity/`). + +Clone repository to: `$HOME/development/pingidentity/` + +```sh +mkdir -p $HOME/development/pingidentity/; cd $HOME/development/pingidentity/ +git clone git@github.com:pingidentity/pingcli.git +... +``` + +To install the CLI for local development, run `make install`. This will build the CLI and install it in your Go binary directory. + +```sh +make install +... +pingcli --version +... +``` + +You can also build without installing: + +```sh +go build -o pingcli . +./pingcli --version +... +``` + +Or run directly from the project root: + +```sh +go run ./ --version +... +``` + +## Testing the CLI + +### Unit Tests + +To run unit tests locally with no external dependencies, you can run `make test`. + +```sh +make test +``` + +### Integration Tests + +To run the full suite of integration tests against live Ping Identity services, you need to set up environment variables and run the complete test suite. + +*Note:* Integration tests interact with real Ping Identity services. Please ensure you have appropriate access to PingOne and PingFederate environments. + +#### Required Environment Variables + +For PingOne integration tests: +```sh +export TEST_PINGONE_ENVIRONMENT_ID="your-environment-id" +export TEST_PINGONE_REGION_CODE="your-region-code" # e.g., "NA", "EU", "AP" +export TEST_PINGONE_WORKER_CLIENT_ID="your-worker-client-id" +export TEST_PINGONE_WORKER_CLIENT_SECRET="your-worker-client-secret" +``` + +For PingFederate container tests: +```sh +export TEST_PING_IDENTITY_DEVOPS_USER="your-devops-username" +export TEST_PING_IDENTITY_DEVOPS_KEY="your-devops-key" +export TEST_PING_IDENTITY_ACCEPT_EULA="YES" +``` + +#### Running Integration Tests + +Run all tests including container setup: +```sh +make devcheck +``` + +Or run tests with an existing container: +```sh +make spincontainer # Start PingFederate container +make test # Run tests +make removetestcontainer # Clean up +``` + +## Using the CLI + +After installing with `make install`, the `pingcli` command will be available in your terminal. + +### Configuration + +Before using the CLI, you need to configure it with your Ping Identity service credentials: + +```sh +pingcli config add-profile +New profile name: : dev +New profile description: : Development environment configuration +Set new profile as active: : y +``` + +Configure your profile with service connection details: + +```sh +# Set PingOne configuration +pingcli config set service.pingone.region "NA" # or "EU", "AP" +pingcli config set service.pingone.environment_id "your-environment-id" +pingcli config set service.pingone.client_id "your-client-id" +pingcli config set service.pingone.client_secret "your-client-secret" + +# Set PingFederate configuration (if needed) +pingcli config set service.pingfederate.host "https://your-pf-host:9999" +pingcli config set service.pingfederate.username "administrator" +pingcli config set service.pingfederate.password "your-password" +``` + +### Testing Your Setup + +Test your configuration by running a simple command: + +```sh +pingcli request --service pingone --http-method GET environments +``` + +## Local SDK Changes + +Occasionally, development may include changes to the [PingOne GO SDK](https://github.com/patrickcping/pingone-go-sdk-v2) or [PingFederate GO Client](https://github.com/pingidentity/pingfederate-go-client). If you'd like to develop the CLI locally using local, modified versions of these SDKs, this can be achieved by adding `replace` directives in the `go.mod` file. + +For example, to use local versions of both SDKs: + +```go +module github.com/pingidentity/pingcli + +go 1.25.1 + +replace github.com/patrickcping/pingone-go-sdk-v2/management => ../pingone-go-sdk-v2/management +replace github.com/patrickcping/pingone-go-sdk-v2/mfa => ../pingone-go-sdk-v2/mfa +replace github.com/pingidentity/pingfederate-go-client/v1220 => ../pingfederate-go-client/v1220 + +require ( + github.com/patrickcping/pingone-go-sdk-v2/management v0.60.0 + github.com/patrickcping/pingone-go-sdk-v2/mfa v0.23.1 + github.com/pingidentity/pingfederate-go-client/v1220 v1220.0.0 + + ... +) + +... +``` + +Once updated, run the following to install the CLI with your local changes: + +```shell +make install +``` + +## Development Tools + +The project includes several development tools to maintain code quality: + +### Code Formatting +```sh +make fmt # Format Go code +make importfmtlint # Format import statements +``` + +### Code Analysis +```sh +make vet # Run go vet +make golangcilint # Run comprehensive linting +``` + +### Development Check +```sh +make devchecknotest # Run all checks except tests +make devcheck # Run all checks including tests +``` diff --git a/contributing/pr-checklist.md b/contributing/pr-checklist.md new file mode 100644 index 00000000..5ddaa16b --- /dev/null +++ b/contributing/pr-checklist.md @@ -0,0 +1,210 @@ +# Pull Request Checklist + +The following provides the steps to check/run to prepare for creating a PR to the `main` branch. PRs that follow these checklists will merge faster than PRs that do not. + +*Note: This checklist is designed to support both human contributors and automated code review tools.* + +## For Automated Code Review + +This checklist includes specific verification criteria marked with *Verification* that can be programmatically checked to support both manual and automated review processes + +## PR Planning & Structure + +- [ ] **PR Scope**. To ensure maintainer reviews are as quick and efficient as possible, please separate support for different features into separate PRs. For example, support for a new `pingcli platform export` service can go in the same PR, however support for new command functionality should be separated. It's acceptable to merge related changes into the same PR where structural changes are being made. + - *Verification*: Check that files modified are logically related (same command directory, related functionality) + +- [ ] **PR Title**. To assist the maintainers in assessing PRs for priority, please provide a descriptive title of the functionality being supported. For example: `Add support for PingFederate export` or `Fix authentication flow for PingOne SSO` + - *Verification*: Title should be descriptive and match the type of changes (Add/Update/Fix/Remove) + +- [ ] **PR Description**. Please follow the provided PR description template and check relevant boxes. Include a clear description of: + - What functionality is being added/changed + - Why the change is needed (e.g., to fix an issue - include the issue number as reference) + - Any breaking changes to CLI commands or configuration + - *Verification*: Check that PR description template boxes are completed and description sections are filled + +## Code Development + +### Architecture & Design + +- [ ] **Code implementation**. New code should follow the established CLI architecture patterns. + - *Verification*: + - New commands are in `cmd//` directories + - Command implementations follow the cobra command pattern + - Internal functionality is organized in `internal/` packages + - Connector implementations are in `internal/connector//` + +- [ ] **SDK Usage**. All Ping Identity API interactions must use the appropriate Go SDKs rather than direct API calls + - *Verification*: + - PingOne API calls use `github.com/patrickcping/pingone-go-sdk-v2/` packages or the `github.com/pingidentity/pingone-go-client/` packages + - PingFederate API calls use `github.com/pingidentity/pingfederate-go-client/` packages + - No direct HTTP calls to Ping Identity APIs (check for `http.Client`, `http.Get`, `http.Post`, etc.) + +### Code Quality + +- [ ] **Installation**. Ensure dependencies are properly maintained and the CLI installs successfully: + +```shell +make install +``` +*Verification*: Run command and verify exit code 0 + +- [ ] **Code Formatting**. Ensure code is properly formatted: + +```shell +make fmt +``` +*Verification*: Run command and verify no files are modified (clean git status) + +- [ ] **Code Linting**. Run all linting checks to ensure code quality and consistency: + +```shell +make vet +make importfmtlint +make golangcilint +``` +*Verification*: Commands must exit with code 0 + +This includes: +- Go vet checks +- golangci-lint for comprehensive static analysis +- Import organization checks + +## Testing + +### Unit Tests + +- [ ] **Unit Tests**. Where a code function performs work internally to a module, but has an external scope (i.e., a function with an initial capital letter `func MyFunction`), unit tests should ideally be created. Not all functions require a unit test, if in doubt please ask: + +```shell +make test +``` +*Verification*: Run command and verify exit code 0 + +### Integration Tests + +- [ ] **Integration Tests**. Where new commands or connectors are being created, or existing functionality is being updated, integration tests should be created or modified according to the [acceptance test strategy](/contributing/acceptance-test-strategy.md) + - *Verification*: + - New commands have corresponding `*_test.go` files in same directory + - Test files follow naming convention `Test_*` or `Test_*` + - Tests include both success and error scenarios + - Connector tests validate API integration functionality + - Configuration dependencies are not hardcoded into the tests (but created as pre-requisites), to allow them to be run by any developer on their own tenant + +- [ ] **Test Environment**. Ensure you have access to a PingOne trial or licensed environment for integration testing. The following environment variables must be set for full test execution: + - `TEST_PINGONE_ENVIRONMENT_ID` + - `TEST_PINGONE_REGION_CODE` + - `TEST_PINGONE_WORKER_CLIENT_ID` + - `TEST_PINGONE_WORKER_CLIENT_SECRET` + + For PingFederate container tests: + - `TEST_PING_IDENTITY_DEVOPS_USER` + - `TEST_PING_IDENTITY_DEVOPS_KEY` + - `TEST_PING_IDENTITY_ACCEPT_EULA=YES` + +- [ ] **Container Tests**. If working with PingFederate functionality, ensure container-based integration tests work properly: + +```shell +make spincontainer +# Run your tests +make removetestcontainer +``` +*Verification*: Container starts healthy and tests execute successfully + +## Documentation + +### Code Documentation + +- [ ] **Command Documentation**. Each cobra command should have appropriate help text and usage examples + - *Verification*: + - All commands have `Short` and `Long` descriptions + - Command flags have clear descriptions + - Complex commands include usage examples in their help text + +- [ ] **Custom Errors**. If required, implement appropriate custom error or warning messages for better user experience when API errors or validation errors occur. Include instruction on how the reader can address the root of the warning or error. Most API level errors do not need custom error handling. + - *Verification*: Custom error functions include actionable guidance for users + +- [ ] **Configuration Documentation**. Changes to configuration options should be documented appropriately + - *Verification*: New configuration keys are documented in `/docs/tool-configuration/configuration-key.md` + +### Examples + +- [ ] **CLI Examples**. New or modified commands should have appropriate usage examples in documentation + - *Verification*: + - Examples exist in relevant documentation files under `docs/` + - Command examples demonstrate both basic and advanced usage + - Examples are tested and work with the current implementation + +- [ ] **Plugin Examples**. If working with plugin functionality, ensure examples are updated in the `examples/plugin/` directory + - *Verification*: Plugin examples compile successfully and demonstrate proper usage patterns + +### Documentation Updates + +- [ ] **README Updates**. If adding new functionality, ensure the main README.md is updated with relevant information + - *Verification*: + - New commands are documented in the Commands section + - Installation instructions remain accurate + - Configuration examples reflect any new requirements + +- [ ] **Documentation Review**. Ensure all documentation changes are clear, accurate, and follow the existing style + - *Verification*: Documentation is well-structured and follows consistent formatting + +## Security & Compliance + +- [ ] **Security Scan**. Ensure your code passes security scanning (this will be automatically checked in CI, but you can run locally if gosec is installed) + - *Verification*: No obvious security issues like hardcoded secrets, unsafe file operations, or command injection vectors + +- [ ] **Sensitive Data**. Ensure no sensitive data (API keys, tokens, etc.) are committed to the repository + - *Verification*: + - No API keys, passwords, or tokens in code or test files + - Sensitive test data uses environment variables + - Configuration files use appropriate masking for sensitive values + - No `.env` files or similar containing credentials + +- [ ] **Input Validation**. Implement appropriate input validation for all user-provided data + - *Verification*: + - Command flags include appropriate validation + - File paths are validated and sanitized + - API inputs are validated before sending to services + +## Final Checks + +- [ ] **All Make Targets**. Run the comprehensive development check (excluding time-intensive tests): + +```shell +make devchecknotest +``` +*Verification*: Run command and verify exit code 0 + +- [ ] **CI Compatibility**. Verify your changes will pass automated CI checks by ensuring all the above steps pass locally + - *Verification*: All previous verification steps completed successfully + +- [ ] **Breaking Changes**. If your PR introduces breaking changes to CLI commands, configuration, or output formats, ensure they are: + - Clearly documented in the PR description + - Included in the changelog entry + - Follow the project's versioning strategy + - *Verification*: + - Breaking changes are documented in PR description + - Migration guidance is provided for users + +## Additional Notes + +- The maintainers may run additional tests in different PingOne regions and with different Ping Identity service configurations +- Large PRs may take longer to review - consider breaking them into smaller, focused changes +- If you're unsure about any step, please ask questions in your PR or create an issue for discussion +- For CLI changes that affect user workflows, consider backwards compatibility and migration paths + +--- + +## Documentation-Only Changes + +If you are making documentation-only changes (guides, examples, or help text), you can use this simplified checklist: + +- [ ] **Guide Updates**. New or updated guides should be clear, well-structured, and include practical CLI examples + +- [ ] **Example Updates**. Ensure any CLI command examples are syntactically correct and demonstrate current functionality + +- [ ] **Help Text Updates**. Verify that command help text is accurate and follows consistent formatting + +- [ ] **Configuration Documentation**. If updating configuration docs, ensure examples match current CLI behavior + +Documentation changes are generally merged quicker than code changes as there is less to review.