Private registries: Add support for Cloudsmith and GCP OIDC configurations#3850
Private registries: Add support for Cloudsmith and GCP OIDC configurations#3850
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds a lightweight schema-based validation layer for private registry credential objects, migrates existing start-proxy auth config parsing to use it, and extends supported OIDC configurations with Cloudsmith + GCP while also propagating the existing replaces-base flag through to proxy outputs.
Changes:
- Introduce a simple JSON schema/validator utility (
validateSchema,optional,FromSchema) plus a test helper for generating schema-shaped objects. - Refactor start-proxy auth config extraction into a dedicated
validation.tsmodule and migrate existing credential types to schema-backed type guards. - Add Cloudsmith and GCP OIDC configuration schemas and include
replaces-basein credential propagation/output.
Show a summary per file
| File | Description |
|---|---|
| src/start-proxy/validation.ts | New auth-config extraction + schema-based cloning/filtering. |
| src/start-proxy/validation.test.ts | Tests for schema-based auth-config extraction and key filtering. |
| src/start-proxy/types.ts | Migrates auth/OIDC types to schema-based validators; adds Cloudsmith/GCP; adds replaces-base. |
| src/start-proxy/types.test.ts | Adds pretty-print coverage for Cloudsmith/GCP and updates secret-hiding tests. |
| src/start-proxy.ts | Switches to new getAuthConfig and propagates replaces-base. |
| src/start-proxy.test.ts | Updates OIDC config tests to iterate over oidcSchemas. |
| src/start-proxy-action.ts | Includes replaces-base in proxy_urls output entries. |
| src/json/testing-util.ts | Adds makeFromSchema test helper for building objects from schemas. |
| src/json/index.ts | Adds schema/validator types + validateSchema implementation. |
| src/json/index.test.ts | Adds unit tests for required vs optional schema validation behavior. |
| lib/start-proxy-action.js | Generated output update (not reviewed). |
Copilot's findings
Comments suppressed due to low confidence (1)
src/start-proxy/types.ts:46
isUsernamePasswordcurrently callsvalidateSchema(usernamePasswordSchema, config), but bothusernameandpasswordare optional in the schema, sovalidateSchemawill returntruefor any object (including OIDC configs). This makes the type guard effectively meaningless/unsound. Consider requiring presence of at least one of the keys (or matching the previous behavior by requiring the relevant key(s) to exist) before returningtrue.
export function isUsernamePassword(
config: AuthConfig,
): config is UsernamePassword {
return json.validateSchema(usernamePasswordSchema, config);
}
- Files reviewed: 10/11 changed files
- Comments generated: 5
68fd7b0 to
0ed734b
Compare
henrymercer
left a comment
There was a problem hiding this comment.
What happens when we merge this? Does the authentication proxy already support these new registries or is there more work to do?
@henrymercer The authentication proxy already supports it (Cloudsmith and GCP). Also, it parses the JSON configuration as a generic map and would ignore keys it doesn't understand, so even if we ended up in a scenario where we have newer configurations than the version of the proxy that we have supports, it shouldn't cause problems. In general, the Dependabot team have been working on the implementation in the proxy and backend. |
henrymercer
left a comment
There was a problem hiding this comment.
Just one thing I missed before. Feel free to merge if you don't think it's important.
This PR primarily adds validation for Cloudsmith and GCP OIDC configurations, so that they can be correctly propagated to the authentication proxy for private package registries when given to the
start-proxyaction as input in GitHub-managed workflows.The first few commits add a lightweight JSON schema implementation to simplify the specification and validation of different credential objects. The existing private registry configuration types are then migrated to make use of this.
We take advantage of this to simplify some of the validation and testing logic so that we have to update fewer places when adding new configuration types.
Then we add the two new configuration types.
Finally, we add validation for the
replaces-basesetting, which has been around for a while, but wasn't accepted until now.I'd suggest reviewing this commit-by-commit.
Risk assessment
For internal use only. Please select the risk level of this change:
Which use cases does this change impact?
Workflow types:
dynamicworkflows (Default Setup, Code Quality, ...).Products:
analysis-kinds: code-scanning.analysis-kinds: code-quality.Environments:
github.comand/or GitHub Enterprise Cloud with Data Residency.How did/will you validate this change?
.test.tsfiles).If something goes wrong after this change is released, what are the mitigation and rollback strategies?
How will you know if something goes wrong after this change is released?
Are there any special considerations for merging or releasing this change?
Merge / deployment checklist