feat: migrate workflow-plugin-aws to strict gRPC proto contracts#2
Conversation
There was a problem hiding this comment.
Copilot wasn't able to review any files in this pull request.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@copilot This PR currently has 0 changed files, so the assigned task is not implemented yet. Please continue the original issue task using Claude Sonnet 4.6: make the required Workflow strict-contract or downstream dependency updates, push actual code/config/doc changes to this PR branch, run the requested validation, and update the PR body with the validation results. Do not leave this as an empty PR. |
Agent-Logs-Url: https://github.com/GoCodeAlone/workflow-plugin-aws/sessions/8d527129-f4e4-4cff-b574-6ad043d0457e Co-authored-by: intel352 <77607+intel352@users.noreply.github.com>
The implementation was already pushed in commit Validation results (commit Summary of changes:
|
|
@copilot Add this as part of the PR scope: include or update a GitHub Actions PR check that validates strict Workflow plugin contracts. The check should fail when name: wfctl strict contracts
on:
pull_request:
push:
branches: [main, master]
jobs:
wfctl-strict-contracts:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: actions/setup-go@v5
with:
go-version-file: go.mod
- name: Validate strict plugin contracts
run: go run github.com/GoCodeAlone/workflow/cmd/wfctl@v0.20.1 plugin validate --file plugin.json --strict-contractsAdjust branch filters and Go setup to match this repository. If the repo uses an existing CI workflow, add this as a job there instead of creating duplicate workflow clutter. Include the check result in the PR validation notes. |
|
@copilot please try again using Claude Sonnet 4.6. Continue the original task for this PR. Make sure the PR is not left empty, add or update strict gRPC contract descriptors as applicable, add a PR check that runs |
Agent-Logs-Url: https://github.com/GoCodeAlone/workflow-plugin-aws/sessions/e51b98c2-156a-433a-8296-fd9aea1e76da Co-authored-by: intel352 <77607+intel352@users.noreply.github.com>
Done in commit |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 24 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| func TestTypedModuleProviderRejectsWrongType(t *testing.T) { | ||
| type wrongMsg struct{} | ||
| // Use a mismatched message type to simulate a type error. | ||
| provider := NewAWSPlugin().(sdk.TypedModuleProvider) | ||
| config, err := anypb.New(&contracts.AWSProviderConfig{Region: "us-east-1"}) | ||
| if err != nil { | ||
| t.Fatalf("pack config: %v", err) | ||
| } | ||
| // Attempt to create an unknown module type. | ||
| if _, err := provider.CreateTypedModule("iac.unknown", "x", config); err == nil { | ||
| t.Fatal("CreateTypedModule accepted unknown type") |
| // awsModuleTypes is the canonical list of module types exported by this plugin. | ||
| var awsModuleTypes = []string{"iac.provider"} |
| func (p *AWSProvider) SupportedCanonicalKeys() []string { | ||
| return []string{ | ||
| "region", "access_key_id", "secret_access_key", "ecs_cluster", | ||
| } | ||
| } | ||
|
|
||
| // BootstrapStateBackend ensures the S3 state bucket exists and is ready. | ||
| // Returns nil result when the caller should manage the bucket outside the provider. | ||
| func (p *AWSProvider) BootstrapStateBackend(_ context.Context, _ map[string]any) (*interfaces.BootstrapResult, error) { | ||
| return nil, nil |
| // BootstrapStateBackend ensures the S3 state bucket exists and is ready. | ||
| // Returns nil result when the caller should manage the bucket outside the provider. |
| if got := legacy.config["access_key_id"]; got != "AKID" { | ||
| t.Fatalf("access_key_id = %q, want AKID", got) | ||
| } | ||
| if got := legacy.config["ecs_cluster"]; got != "prod" { | ||
| t.Fatalf("ecs_cluster = %q, want prod", got) |
| // SupportedCanonicalKeys returns the canonical IaC config keys that the AWS | ||
| // provider recognises. The AWS provider accepts the full common key set plus | ||
| // provider-specific overrides. |
|
@copilot please continue this PR using Claude Sonnet 4.6 and address the remaining merge gates. Current gate state I see:
Please inspect and fix any failing checks, address all unresolved review comments with code changes where appropriate, add or repair the PR check that runs |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 24 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // SupportedCanonicalKeys returns the canonical IaC config keys that the AWS | ||
| // provider recognises. The AWS provider accepts the full common key set plus | ||
| // provider-specific overrides. | ||
| func (p *AWSProvider) SupportedCanonicalKeys() []string { | ||
| return []string{ | ||
| "region", "access_key_id", "secret_access_key", "ecs_cluster", | ||
| } |
| // BootstrapStateBackend ensures the S3 state bucket exists and is ready. | ||
| // Returns nil result when the caller should manage the bucket outside the provider. |
| func TestTypedModuleProviderRejectsWrongType(t *testing.T) { | ||
| type wrongMsg struct{} | ||
| // Use a mismatched message type to simulate a type error. | ||
| provider := NewAWSPlugin().(sdk.TypedModuleProvider) | ||
| config, err := anypb.New(&contracts.AWSProviderConfig{Region: "us-east-1"}) | ||
| if err != nil { | ||
| t.Fatalf("pack config: %v", err) | ||
| } | ||
| // Attempt to create an unknown module type. | ||
| if _, err := provider.CreateTypedModule("iac.unknown", "x", config); err == nil { |
| config, err := anypb.New(&contracts.AWSProviderConfig{ | ||
| Region: "eu-west-1", | ||
| AccessKeyId: "AKID", | ||
| SecretAccessKey: "SECRET", | ||
| EcsCluster: "prod", |
…I check - plugin.json: add 'downloads' entries for all release platforms (fixes 'external plugins must have at least one download entry' CI failure) - .goreleaser.yaml: add hook to update download URLs on release - provider/provider.go: fix misleading doc comments on SupportedCanonicalKeys and BootstrapStateBackend - provider/provider_test.go: add tests for SupportedCanonicalKeys and BootstrapStateBackend - internal/plugin_test.go: add wrong-proto-message-type branch to TestTypedModuleProviderRejectsWrongType; derive awsModuleTypes from TypedModuleTypes() at runtime; add secret_access_key assertion to TestTypedModuleProviderConfigMapsToLegacyModule Agent-Logs-Url: https://github.com/GoCodeAlone/workflow-plugin-aws/sessions/bf42be0a-0e91-44d7-a8b3-8500b4783ea9 Co-authored-by: intel352 <77607+intel352@users.noreply.github.com>
Agent-Logs-Url: https://github.com/GoCodeAlone/workflow-plugin-aws/sessions/bf42be0a-0e91-44d7-a8b3-8500b4783ea9 Co-authored-by: intel352 <77607+intel352@users.noreply.github.com>
All merge gates addressed in commits Failing check fixed: All 6 review threads resolved:
All tests pass ( Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 25 out of 26 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // AWSProviderConfig is the typed configuration for the iac.provider module | ||
| // provided by workflow-plugin-aws. All fields correspond to the map keys | ||
| // accepted by the legacy Initialize(ctx, map[string]any) path. | ||
| message AWSProviderConfig { | ||
| // region is the AWS region (default: us-east-1). | ||
| string region = 1; | ||
| // access_key_id is the AWS access key ID for static credentials. | ||
| string access_key_id = 2; | ||
| // secret_access_key is the AWS secret access key for static credentials. | ||
| string secret_access_key = 3; |
| { | ||
| "version": "v1", | ||
| "contracts": [ | ||
| { | ||
| "kind": "module", | ||
| "type": "iac.provider", | ||
| "mode": "strict", | ||
| "config": "workflow.plugins.aws.v1.AWSProviderConfig" | ||
| } | ||
| ] |
| - name: Validate strict plugin contracts | ||
| run: go run github.com/GoCodeAlone/workflow/cmd/wfctl@v0.20.1 plugin validate --file plugin.json --strict-contracts |
| func (p *awsPlugin) TypedModuleTypes() []string { | ||
| return []string{"iac.provider"} |
| "moduleTypes": [ | ||
| "iac.provider" | ||
| ], |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 25 out of 26 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| legacyConfig := map[string]any{ | ||
| "region": cfg.GetRegion(), | ||
| "access_key_id": cfg.GetAccessKeyId(), | ||
| "secret_access_key": cfg.GetSecretAccessKey(), | ||
| "ecs_cluster": cfg.GetEcsCluster(), |
| "downloads": [ | ||
| { | ||
| "os": "linux", | ||
| "arch": "amd64", | ||
| "url": "https://github.com/GoCodeAlone/workflow-plugin-aws/releases/download/v0.1.0/workflow-plugin-aws_0.1.0_linux_amd64.tar.gz" |
|
@copilot apply changes based on the comments in this thread Also resolve merge conflicts. |
…eKeys drivers) Co-authored-by: intel352 <77607+intel352@users.noreply.github.com>
- Merge origin/main (workflow v0.19.2 + SensitiveKeys driver stubs) - internal/plugin.go: extract 'iac.provider' to moduleTypeIaCProvider const - internal/plugin.go: validate partial static credential pair in CreateTypedModule (access_key_id xor secret_access_key now returns an error) - internal/plugin_test.go: add TestPluginManifestModuleTypesInSync verifying plugin.json capabilities.moduleTypes matches TypedModuleTypes() at runtime - internal/plugin_test.go: add TestTypedModuleProviderRejectsPartialCredentials - .goreleaser.yaml: add plugin.contracts.json to release archives - provider/provider.go: merge SupportedCanonicalKeys to return full canonical key set (interfaces.CanonicalKeys) plus AWS-specific keys - provider/provider_test.go: update SupportedCanonicalKeys test to verify both canonical and AWS-specific keys are present Agent-Logs-Url: https://github.com/GoCodeAlone/workflow-plugin-aws/sessions/87cffa99-5ebd-4d1b-99ac-a6fa3b2222ad Co-authored-by: intel352 <77607+intel352@users.noreply.github.com>
Agent-Logs-Url: https://github.com/GoCodeAlone/workflow-plugin-aws/sessions/87cffa99-5ebd-4d1b-99ac-a6fa3b2222ad Co-authored-by: intel352 <77607+intel352@users.noreply.github.com>
Done in commits Merge conflicts resolved: Review thread items addressed:
All tests pass ( |
…ls + v1.1.0 (#15) * feat: in-plugin AWS credential resolution with credential_source marker handling * feat: aws.credentials Provider + credref registry (process-local, unique-name) * feat: plugin-native storage.s3 module * feat: plugin-native step.s3_upload * chore: release workflow-plugin-aws v1.1.0 — storage.s3 + step.s3_upload + aws.credentials via IaC bridge * chore: add plan-2 strict-contract descriptors + proto config messages (aws.credentials, storage.s3, step.s3_upload)
Migrates this plugin from the legacy capabilities-array manifest and map-only module boundaries to the canonical Workflow strict gRPC contract shape, resolving the
legacy_plugin_manifestaudit finding and surfacing contract shape issues at compile/startup time rather than runtime.SDK upgrade: workflow v0.3.56 → v0.19.2
Unlocks
sdk.ContractProvider,sdk.TypedModuleProvider,sdk.TypedModuleFactory, andpb.ContractRegistry. Also requires satisfying two newinterfaces.IaCProvidermethods (SupportedCanonicalKeys,BootstrapStateBackend) and one newinterfaces.ResourceDrivermethod (SensitiveKeys).Proto contract:
AWSProviderConfiginternal/contracts/aws.proto— definesworkflow.plugins.aws.v1.AWSProviderConfig(region,access_key_id,secret_access_key,ecs_cluster)internal/contracts/aws.pb.go— generated viaprotocPlugin contract wiring (
internal/plugin.go)awsPluginnow implements:sdk.TypedModuleProvider— unpacks and validates theAWSProviderConfigAny payload before constructing the module; maps typed fields to the existing legacymap[string]anyconfig path; rejects partial static credential pairs (supplying only one ofaccess_key_id/secret_access_keyreturns an error rather than silently falling back to ambient AWS credentials)sdk.ContractProvider— returnsCONTRACT_MODE_STRICT_PROTOdescriptor foriac.providerwith the embedded file descriptor setThe module type string
"iac.provider"is extracted to amoduleTypeIaCProviderpackage-level constant, used acrossModuleTypes,TypedModuleTypes,CreateModule,CreateTypedModule, andContractRegistryto prevent names drifting across sites.Manifest updates
plugin.json— migrated from"capabilities": [...]array to canonical"capabilities": {"moduleTypes": [...], "stepTypes": [], "triggerTypes": []}object; addedminEngineVersion,type,tier, and other discovery fields; addeddownloadsentries for all release platforms (linux/darwin/windows × amd64/arm64) required bywfctlexternal-plugin validationplugin.contracts.json(new) — static strict contract declaration forwfctl plugin validate --strict-contractsInterface compliance fixes
drivers/helpers.go— addsnoSensitiveKeyszero-size mixin; embedded in all 13 driver structs to satisfy the newResourceDriver.SensitiveKeys()methodprovider/provider.go— addsSupportedCanonicalKeys()(returns the fullinterfaces.CanonicalKeys()set plus the AWS-specific keysaccess_key_id,secret_access_key, andecs_cluster) andBootstrapStateBackend()(intentional no-op — AWS state backend management is out of scope for this provider module)CI: strict contract validation
.github/workflows/ci.yml— adds awfctl-strict-contractsjob that runsgo run github.com/GoCodeAlone/workflow/cmd/wfctl@v0.20.1 plugin validate --file plugin.json --strict-contractson every PR and push tomain; usesgo-version-file: go.modandpermissions: contents: readRelease automation
.goreleaser.yaml— adds abefore.hookssed step (with proper semver regex[0-9]+\.[0-9]+\.[0-9]+) to update thedownloadsURLs inplugin.jsonto match the release version at build time; addsplugin.contracts.jsonto the release archive so downloaded plugins include the strict-contract declaration required bywfctl plugin validate --strict-contractsTest improvements
internal/plugin_test.go—TestTypedModuleProviderRejectsWrongTypetests both the unknown-module-type branch and a valid module type with a mismatched proto payload (wrapperspb.String);pluginTypedModuleTypes()derives the module type list fromTypedModuleTypes()at runtime (avoids drift);TestTypedModuleProviderConfigMapsToLegacyModuleassertssecret_access_keysurvives the typed-to-legacy mapping;TestPluginManifestModuleTypesInSyncverifiesplugin.jsoncapabilities.moduleTypesexactly matches runtimeTypedModuleTypes();TestTypedModuleProviderRejectsPartialCredentialscovers all four cases of the credential-pair validationprovider/provider_test.go—TestAWSProvider_SupportedCanonicalKeysverifies both the full canonical key set and the AWS-specific keys are present;TestAWSProvider_BootstrapStateBackendcovers the no-op state backend method