Skip to content

Simplify: references to avoid hardcoding and confusion#1000

Draft
harshadixit12 wants to merge 2 commits into
mainfrom
fix/standardise-references
Draft

Simplify: references to avoid hardcoding and confusion#1000
harshadixit12 wants to merge 2 commits into
mainfrom
fix/standardise-references

Conversation

@harshadixit12
Copy link
Copy Markdown
Contributor

@harshadixit12 harshadixit12 commented May 7, 2026

Rather than hardcoding ID as "" and "[unknown]", refactoring to use constants, and functions for initialising the types. This will make the code more readable.

@harshadixit12 harshadixit12 temporarily deployed to kongctl-acceptance-3 May 7, 2026 19:44 — with GitHub Actions Inactive
@harshadixit12 harshadixit12 temporarily deployed to kongctl-acceptance-4 May 7, 2026 19:44 — with GitHub Actions Inactive
@harshadixit12 harshadixit12 temporarily deployed to kongctl-acceptance May 7, 2026 19:44 — with GitHub Actions Inactive
@harshadixit12 harshadixit12 temporarily deployed to kongctl-acceptance-5 May 7, 2026 19:44 — with GitHub Actions Inactive
@harshadixit12 harshadixit12 temporarily deployed to kongctl-acceptance-2 May 7, 2026 19:44 — with GitHub Actions Inactive
@harshadixit12 harshadixit12 marked this pull request as ready for review May 8, 2026 04:58
@harshadixit12 harshadixit12 requested review from a team as code owners May 8, 2026 04:58
Copilot AI review requested due to automatic review settings May 8, 2026 04:58
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors declarative planner/executor reference handling by replacing hardcoded sentinel strings ("", "[unknown]") with named constants and helper constructors/methods, to reduce ambiguity and improve readability across planning, dependency resolution, and execution.

Changes:

  • Introduce RefIDPendingCreation / RefIDPendingLookup plus helper constructors and IsResolved/NeedsResolution methods for reference/parent IDs.
  • Update planners and the reference resolver to use the new helpers instead of hardcoded sentinel strings.
  • Update dependency resolution and executor logic/tests to use the new semantics consistently.

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
internal/declarative/planner/types.go Adds reference/parent sentinel constants plus helper constructors and resolution methods.
internal/declarative/planner/resolver.go Replaces "[unknown]" with RefIDPendingCreation in reference resolution.
internal/declarative/planner/portal_planner.go Uses NewPendingLookupRef for portal auth-strategy references.
internal/declarative/planner/planner.go Merges resolver results into existing ReferenceInfo while preserving lookup fields.
internal/declarative/planner/event_gateway_virtual_cluster_planner.go Uses NewPendingLookupRef for gateway ID resolution.
internal/declarative/planner/event_gateway_tls_trust_bundle_planner.go Uses NewPendingLookupRef for gateway ID resolution.
internal/declarative/planner/event_gateway_static_key_planner.go Uses NewPendingLookupRef for gateway ID resolution.
internal/declarative/planner/event_gateway_schema_registry_planner.go Uses NewPendingLookupRef for gateway ID resolution.
internal/declarative/planner/event_gateway_listener_planner.go Uses NewPendingLookupRef for gateway ID resolution.
internal/declarative/planner/event_gateway_data_plane_certificate_planner.go Uses NewPendingLookupRef for gateway ID resolution.
internal/declarative/planner/event_gateway_backend_cluster_planner.go Uses NewPendingLookupRef for gateway ID resolution.
internal/declarative/planner/dependencies.go Switches dependency checks to IsResolved/NeedsResolution helpers.
internal/declarative/planner/dependencies_test.go Updates tests to use RefIDPendingCreation.
internal/declarative/executor/portal_operations_test.go Updates executor portal test to use planner.RefIDPendingCreation.
internal/declarative/executor/executor.go Centralizes unresolved-ID checks via constants and NeedsResolution/IsResolved.
internal/declarative/executor/executor_test.go Updates executor tests to use planner.RefIDPendingCreation.

Comment thread internal/declarative/planner/types.go Outdated
Comment thread internal/declarative/planner/resolver.go Outdated
@harshadixit12 harshadixit12 temporarily deployed to kongctl-acceptance-3 May 8, 2026 05:11 — with GitHub Actions Inactive
@harshadixit12 harshadixit12 temporarily deployed to kongctl-acceptance-5 May 8, 2026 05:11 — with GitHub Actions Inactive
@harshadixit12 harshadixit12 temporarily deployed to kongctl-acceptance-2 May 8, 2026 05:11 — with GitHub Actions Inactive
@harshadixit12 harshadixit12 temporarily deployed to kongctl-acceptance-4 May 8, 2026 05:11 — with GitHub Actions Inactive
@harshadixit12 harshadixit12 temporarily deployed to kongctl-acceptance May 8, 2026 05:11 — with GitHub Actions Inactive
@harshadixit12 harshadixit12 marked this pull request as draft May 8, 2026 09:34
@rspurgeon rspurgeon changed the title refactor: references to avoid hardcoding and confusion Simplify: references to avoid hardcoding and confusion May 8, 2026
@rspurgeon
Copy link
Copy Markdown
Collaborator

@harshadixit12 please close or revive this PR when you are able.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants