feat(configuration_v2): carry inline type/parameters on connector, processor_group and inner processors#211
Draft
jamesmewton wants to merge 1 commit into
Draft
Conversation
…ocessor_group and inner processors
Adds schema and wiring so inline components on bindplane_configuration_v2
round-trip through terraform apply with their identity intact:
- connector block: new type and parameters_json attributes
- processor_group block: new parameters_json attribute for fields such
as telemetry_types
- processor_group block: new processor block (singular) for inline
inner processors, carrying name + type + parameters_json alongside
the existing processors name list
Previously ResourceConfig carried only Name, Processors, RouteID and
Routes through withResourcesByName, so inline Type / Parameters on
connectors, processor groups, and inner processors were dropped on
apply. This caused the SaaS UI to render pipelines with inline
components as disconnected processor groups.
Schema is non-breaking: existing processors = [names] keeps working,
the new processor block is additive. Read path reclassifies inner
processors on Parameters non-empty (rather than Type) to avoid
spurious drift when the server enriches a library-referenced processor
with Type that the user did not declare. Same classification is
applied to connectors for symmetry.
Follows the type + parameters_json pattern already used by the
standalone bindplane_connector resource.
Closes observIQ#210
jsirianni
requested changes
May 7, 2026
jsirianni
left a comment
Member
There was a problem hiding this comment.
Hi @jamesmewton. I appreciate the pull request. Would you be able to break it up into several smaller, focused changes?
We have been super busy lately, it will be easier to review smaller changes. Overall, I think this looks good, and I see no reason to reject them.
|
Hi @jsirianni No problems will split this up into smaller changes and advise |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #210.
Adds schema and wiring so inline components on
bindplane_configuration_v2round-trip throughterraform applywithout losing their identity. Covers the three wiring-critical field categories that are currently stripped (connectortype+parameters, processor_groupparameters, inner processortype+parameters), which in our testing is everything needed to restore SaaS pipeline rendering for the cases we've seen in the wild.What's in this PR
internal/configuration/configuration.goResourceConfig:TypeandParameters, populated ontomodel.ResourceConfiguration.ParameterizedSpecinwithResourcesByName.ProcessorRefstruct andProcessorRefs []ProcessorReffield onResourceConfig, appended into the same inner processor list as the existing name-onlyProcessors.provider/resource_configuration_v2.goconnectorblock: newtype(string, optional) andparameters_json(string, optional,DiffSuppressFunc: suppressEquivalentJSONDiffs).processor_groupblock: newparameters_json(for fields liketelemetry_types).processor_groupblock: newprocessorblock (singular) for inline inner processors withname(required) +type(optional) +parameters_json(optional).processors = [names]is kept as an additive non-breaking alternative and relaxed from Required to Optional.bindplane_connectorresource so the mental model stays consistent across resource types.internal/configuration/configuration_test.goTestWithConnectorsByName_InlineTypeAndParametersTestWithProcessorGroups_ParametersTestWithProcessorGroups_InlineProcessorRefsTestWithProcessorGroups_MixedProcessorsAndRefsdocs/resources/bindplane_configuration_v2.mdConnector Blocksection (it wasn't in the options reference previously).processorblock nested underprocessor_group.routing:3connector, an inlinebatch:3processor on one processor group, and the name-only form on another.Schema design notes
A couple of choices worth flagging for review:
Inner processors as an additive
processorblock rather than breaking theprocessorsstring list. Breaking the list form would have been simpler in the wiring but would force every existing user to rewrite their TF. The additive block keeps everyone's current config working unchanged. Happy to go the other way if you'd prefer to break it in a major.processorsrelaxed from Required to Optional. Needed so users can express a processor group using only theprocessorblock form without a stub empty list. Strictly additive — no existing config breaks.Read-path classification uses
len(Parameters) > 0, not the presence ofType. When a library-referenced processor or connector hits the server, the server commonly populatesTypefrom the referenced resource. If we reclassified onType != "", that would flip aprocessors = ["my-filter"]config into aprocessor { name = ..., type = ... }block on every refresh and create spurious drift. UsingParametersas the discriminator avoids that: library-referenced components stay in the simpler form; inline ones (which always have Parameters in practice for routing/batch/etc.) round-trip through the rich form. Same logic applied to connectors for symmetry. Happy to discuss state-aware Read logic if you'd prefer a more principled approach.Known limitations
Calling these out explicitly rather than hiding them:
typebut noparameters_jsonwill be reclassified as library-referenced on Read and thetypefield cleared from state, causing drift. In practice this affects anyone who declarestype = "batch:3"with no parameters (relying on defaults). Workaround: declare at least one parameter, or use theprocessors = [names]list form. The drift-avoidance trade-off described in design note 3 is why. Open to a state-aware Read if you'd rather avoid this.parameters_jsondrift on"" vs "[]".suppressEquivalentJSONDiffstreats only-one-empty as different, so users who declareparameters_json = jsonencode([])(an explicit empty parameters list) will see drift since the Read path returns""for empty parameters. Users who omit the attribute entirely are unaffected. Minor — happy to extend the diff-suppress if you want it bundled in here.displayNameis not covered. The related restoration we had to run as a workaround also re-patcheddisplayNameon inner processors; this PR does not carry it through. The effect is cosmetic only — the SaaS canvas falls back to the internal name. PR feat: Support resource display name and description metadata fields #115 addsdisplay_nameon standalone resources; I didn't want this PR to collide with that surface, sodisplay_nameon inline inner processors is deliberately a separate follow-up.What's explicitly out of scope
display_namesupport on any resource. PR feat: Support resource display name and description metadata fields #115 is in flight on the standalone-resource surface; inner-processordisplay_namecan pick up from there.processorblock support onsourceanddestination. The defect we've been chasing is scoped to processor_group inner processors, so I scoped the same way. Happy to extend if you'd like symmetry.Testing
internal/configurationfor the new plumbing, following the style ofTestReadRolloutOptions/ the existing table-drivenTestNewV2tests.gofmtclean on the four files touched.go build/go testlocally:github.com/observiq/bindplane-op-enterpriseis private and I don't have SSH access. CI has the secret and will validate. If there's a documented path for external contributors to build this locally, happy to follow it; otherwise I'll iterate on CI output and review comments.type=routing:2+ two parameters; processor group withtelemetry_typesparameter + an inner processor withtype=batch:2+ two parameters), GETed it back, and verified every field round-trips. 0 failures acrossconnector.type,connector.parameters,processor_group.parameters,inner_processor.type,inner_processor.parameters. This confirms BindPlane accepts and preserves the shape; it does not reproduce the SaaS-specific render bug (localhost OP renders specs more leniently than SaaS), which is a distinct layer.bindplane_configuration_v2. I don't have a way to run those locally.Evidence
We hit this on a couple of production Bindplane Cloud configs with inline
routing:3connectors and a few inlinebatch:3processors embedded on processor groups (~50 processor groups across 30+ routes on the largest). Afterterraform apply, the SaaS UI rendered the downstream pipeline as disconnected processor groups with no incoming edges. Library-referenced components on the same config deployed fine.We confirmed the root cause by running a
null_resourcepost-applylocal-execthat re-PATCHes the stripped fields viaPOST /v1/apply: with connectortype/parameters, processor grouptelemetry_types, and inner processortype/parametersrestored, the SaaS UI resolves everything correctly and the pipeline renders 1:1 with the original UI config. HAR captures of the SaaS graph before and after the workaround made the causal chain unambiguous. The example in the docs is a synthetic minimal repro; happy to share more specific patterns privately if it would help review.Compatibility
Optionaland all changes are additive. Existingbindplane_configuration_v2configs that don't set the new fields are unaffected.processorsonprocessor_groupis relaxed from Required to Optional. Existing configs that setprocessors = [...]continue to work; new configs can omit it when usingprocessorblocks instead.suppressEquivalentJSONDiffsprevents spurious drift on JSON re-serialisation, matching the standalone resource.Happy to iterate on naming, descriptions, schema shape, or anything else on review.