feat: private workflow registry deployment behind a feature flag#367
feat: private workflow registry deployment behind a feature flag#367
Conversation
39b44d6 to
a4048aa
Compare
| // resolveWorkflowOwner returns the effective owner address for workflow ID computation. | ||
| // For private registry deploys, the owner is derived from tenantID and organizationID. | ||
| // For onchain deploys, the configured WorkflowOwner address is used directly. | ||
| func (h *handler) resolveWorkflowOwner(targetWorkflowRegistry registryTarget) (string, error) { |
There was a problem hiding this comment.
Temporary implementation, that will be resolved with derived workflow owner from GQL in the future.
There was a problem hiding this comment.
Ah, I was going to note this. Would we add a new resolveOwner endpoint? From what I can see this flow does not support API_KEY yet, which is fine for now but will need to be supported eventually
There was a problem hiding this comment.
We discussed offline, we will use derieved owner from API call, part of a follow up work.
There was a problem hiding this comment.
Pull request overview
This PR introduces a preview “private workflow registry” deploy path (STAGING-only) and refactors workflow deployment to route through registry-specific strategies (onchain vs private), with accompanying unit and e2e coverage.
Changes:
- Add
--preview-private-registryflag and resolve registry target (private vs onchain) with STAGING gating. - Refactor deploy flow into
registryDeployStrategyimplementations for onchain and private registry upsert behavior. - Add unit tests + multi-command e2e happy path for private registry deployment.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/multi_command_test.go | Adds multi-command happy path case for private registry deploy preview flag. |
| test/multi_command_flows/workflow_private_registry.go | New e2e flow mocking GraphQL + upload endpoints to validate private registry deploy output. |
| cmd/workflow/deploy/registry_deploy_strategy_test.go | Unit tests for registry target resolution and STAGING-only gating. |
| cmd/workflow/deploy/registry_deploy_strategy_private.go | Implements private registry deploy strategy using GraphQL upsert + details output. |
| cmd/workflow/deploy/registry_deploy_strategy_onchain.go | Extracts onchain deploy logic into a strategy with async WRC initialization and prechecks. |
| cmd/workflow/deploy/registry_deploy_strategy.go | Adds target resolution + strategy factory and STAGING-only validation for preview. |
| cmd/workflow/deploy/private_registry_test.go | Adds tests for private-registry inputs mapping and owner derivation behavior. |
| cmd/workflow/deploy/prepare.go | Updates workflow artifact preparation to accept an explicit workflow owner for WorkflowID generation. |
| cmd/workflow/deploy/deploy_test.go | Adds tests for resolving private registry target/owner and executing private deploy path via mocked GraphQL. |
| cmd/workflow/deploy/deploy.go | Adds preview flag + target selection, derives owner for private registry, and orchestrates deploy via strategies. |
| cmd/workflow/deploy/compile_test.go | Updates tests for the new PrepareWorkflowArtifact(workflowOwner) signature. |
| cmd/workflow/deploy/autoLink.go | Uses resolved h.inputs.WorkflowOwner for autolink inputs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…to DEVSVCS-4615/private-registry-deploy
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| func createTestBearerCredentialsHome(t *testing.T) string { | ||
| t.Helper() | ||
|
|
||
| homeDir := t.TempDir() | ||
| creDir := filepath.Join(homeDir, ".cre") | ||
| require.NoError(t, os.MkdirAll(creDir, 0o700), "failed to create .cre dir") | ||
|
|
||
| jwt := createTestJWT("test-org-id") | ||
| creConfig := "AccessToken: " + jwt + "\n" + | ||
| "IDToken: test-id-token\n" + | ||
| "RefreshToken: test-refresh-token\n" + | ||
| "ExpiresIn: 3600\n" + | ||
| "TokenType: Bearer\n" | ||
|
|
||
| require.NoError(t, os.WriteFile(filepath.Join(creDir, "cre.yaml"), []byte(creConfig), 0o600), "failed to write test credentials") | ||
|
|
||
| return homeDir | ||
| } |
There was a problem hiding this comment.
The private registry test needs to override HOME (and consequently GOPATH) because it uses bearer token (JWT) authentication rather than the API key authentication used by the other happy-path e2e tests.
Why HOME must be overridden:
The other on-chain/EOA deploy tests authenticate by simply setting an environment variable (t.Setenv(credentials.CreApiKeyVar, "test-api")). The CLI reads that env var directly — no files on disk involved, no need to touch HOME.
The private registry flow authenticates using bearer credentials stored in a file at ~/.cre/cre.yaml. The test must fabricate a fake cre.yaml containing a synthetic JWT (with the right org_id, exp, etc.) so the CLI subprocess can read it. To make the subprocess find the fake credentials file (instead of the real user's ~/.cre/cre.yaml, or fail because none exists), HOME is redirected to a t.TempDir() where the fake .cre/cre.yaml lives.
Why GOPATH must be pinned:
When HOME is overridden to a temp directory, Go's default GOPATH becomes $HOME/go — which now points inside the temp dir (e.g. /tmp/.../go). The Go module cache under GOPATH contains read-only files. When the test finishes, t.TempDir() cleanup tries to remove the entire temp directory tree. It cannot delete the read-only module cache files, which causes the cleanup to fail and marks the test as failed even though the actual test logic succeeded. By explicitly setting GOPATH back to the real home's go directory (e.g. /Users/jnowak/go), the module cache stays outside the temp dir and cleanup succeeds cleanly.
In short: the on-chain tests inject auth via a simple env var, but the private registry test injects auth via a file on disk, which forces the HOME override, which in turn necessitates the GOPATH fix to avoid a spurious cleanup failure.
timothyF95
left a comment
There was a problem hiding this comment.
Can address in follow up PR
| // resolveWorkflowOwner returns the effective owner address for workflow ID computation. | ||
| // For private registry deploys, the owner is derived from tenantID and organizationID. | ||
| // For onchain deploys, the configured WorkflowOwner address is used directly. | ||
| func (h *handler) resolveWorkflowOwner(targetWorkflowRegistry registryTarget) (string, error) { |
There was a problem hiding this comment.
Ah, I was going to note this. Would we add a new resolveOwner endpoint? From what I can see this flow does not support API_KEY yet, which is fine for now but will need to be supported eventually
|
|
||
| const ( | ||
| registryTargetOnchain registryTargetType = "onchain" | ||
| registryTargetPrivate registryTargetType = "private" |
There was a problem hiding this comment.
Can we reuse the registry types already defined by tenant manifest?
cre-cli/internal/tenantctx/tenantctx.go
Line 121 in 2c7e301
We could hoist as an enum or const as you suggested in #371 (comment)
There was a problem hiding this comment.
This part should be refactored in https://smartcontract-it.atlassian.net/browse/DEVSVCS-4634. I guess the flow control will change and use fields directly from resolved registry.
* registry client * tests * linter * initial implementation of private workflow registry deploy * refactoring * fix derived owner * init refactoring * move workflow exists function * merge input to usage * remove not neeeded check * merge registry target and adapter logic * consolidate files * refactor deploy inputs * rename * add unit tests * init e2e test * improve test * remove redundant file * rename * linter * fix wornflow owner * Update cmd/workflow/deploy/artifacts_test.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update test/multi_command_flows/workflow_private_registry.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * code review fixes * docs * fix test * update doc for temp feature flag --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
cre workflow deploybehind--preview-private-registry(STAGING-only).onchainvsprivate) and routed pre-checks/upsert through a shared interface.