[CXP-637] fix capabilities workflow failing with missing --input flag#44
Conversation
Connector PR Review: [CXP-637] fix capabilities workflow failing with missing --input flagBlocking Issues: 0 | Suggestions: 0 | Threads Resolved: 0 Review SummaryThe full PR diff was scanned for security and correctness. The new commit only changes StaticCapabilitiesConnector.Metadata DisplayName from "Baton-File Connector" to "File Connector" (pkg/connector/connector.go:36), which now matches the existing FileConnector.Metadata display name, a safe non-breaking display-name change (B9). No new issues found. Security IssuesNone found. Correctness IssuesNone found. SuggestionsNone. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
could you generate capabilities from main? https://github.com/ConductorOne/baton-file/blob/main/cmd/baton-file/main.go#L28
if you remove this line from baton admin and add WithDefaultCapabilitiesConnectorBuilder you don't need this file anymore
There was a problem hiding this comment.
Good call on the direction. One thing to flag though: passing &FileConnector{} directly won't work here — ResourceSyncers() tries to load the input file and returns nil syncers when it's empty, so you'd get capabilities with zero resource types. Since baton-file is data-driven, you'd need a small StaticCapabilitiesConnector that just iterates TraitMap without touching any file:
type StaticCapabilitiesConnector struct{}
func (s *StaticCapabilitiesConnector) ResourceSyncers(ctx context.Context) []connectorbuilder.ResourceSyncerV2 {
var syncers []connectorbuilder.ResourceSyncerV2
for name, trait := range TraitMap {
syncers = append(syncers, &resourceBuilder{resourceType: buildDynamicResourceType(name, trait.String())})
}
return syncers
}Then in main.go: connectorrunner.WithDefaultCapabilitiesConnectorBuilder(&connector.StaticCapabilitiesConnector{}). With that, ./connector capabilities works with no --input and the whole fixture machinery in this workflow goes away. Worth doing in this PR or as a follow-up?
| ./connector config > config_schema.json | ||
| # Assert all known traits appear in the output. If TraitMap grows, add the | ||
| # new trait to both the fixture above and this list. | ||
| for trait in TRAIT_USER TRAIT_GROUP TRAIT_ROLE TRAIT_APP TRAIT_SECRET; do |
There was a problem hiding this comment.
nit: this assertion loop lives inside the Run and save output step, so when a trait is missing CI shows Run and save output failed — not super helpful for diagnosing what's actually broken. Would be cleaner to pull the for loop into its own step:
- name: Verify trait coverage in capabilities output
run: |
for trait in TRAIT_USER TRAIT_GROUP TRAIT_ROLE TRAIT_APP TRAIT_SECRET; do
grep -q "$trait" baton_capabilities.json || { echo "ERROR: $trait missing from baton_capabilities.json"; exit 1; }
doneThat said, if luisina's WithDefaultCapabilitiesConnectorBuilder approach gets implemented, this whole step goes away anyway — so might not be worth fixing here if that's coming soon.
FeliLucero1
left a comment
FeliLucero1
left a comment
There was a problem hiding this comment.
lgtm! the StaticCapabilitiesConnector approach is the right call — clean and no fixture needed.
two small nits inline:
- display name inconsistency between StaticCapabilitiesConnector ("Baton-File Connector") and FileConnector ("File Connector") — probably worth aligning
- baton_capabilities.json and config_schema.json are now committed with no workflow to regenerate them — is baton-admin running the binary directly and these files are just snapshots, or do they need a new update mechanism?
4 participants
Description
Useful links: