Remove remaining VIP/BGP/IPAM remnants by piwi3910 · Pull Request #890 · azrtydxb/novaedge

piwi3910 · 2026-03-07T13:57:14Z

Summary

Second pass cleanup after PR #889. Removes all remaining VIP, BGP, OSPF, BFD, and IPAM references from the entire codebase.

Proto: Removed VIPAssignment, VIPMode, BGPConfig, OSPFConfig, BFDConfig messages and VIP RPCs from config, dataplane, and federation protos. Reserved field numbers for backward compatibility.
API types: Removed VIPRef from ProxyGatewaySpec.
Controller: Removed VIP ref handling from ingress translator, --default-vip-ref flag, DefaultVIPRef constant, getVIPRef() method, and NewIngressTranslatorWithOptions VIP parameter.
Go dataplane: Removed translateVIPs/translateVIPMode, UpsertVIP/DeleteVIP client methods.
Rust dataplane: Removed upsert_vip/delete_vip gRPC handlers, VIP apply logic in apply_config, VIP status reporting, and the entire vip/ module (garp, manager, routing).
CLI (novactl): Removed VIP/IPPool resource aliases, agent VIP query, metrics VIPs subcommand, VIP/IPPool printer functions.
Tests: Removed all VIP-related test functions and VIP data from snapshot test fixtures across 11 test files.
Config/Helm: Removed vipRef from 14 sample YAML files, deleted ProxyVIP CRD, cleaned RBAC and kustomization.

Verification

go build ./... passes
go vet ./... passes
golangci-lint run ./... -- 0 issues
go test ./... passes (1 pre-existing flaky gossip test)
helm lint charts/novaedge charts/novaedge-agent charts/novaedge-operator -- 0 failures
cargo check / cargo clippy / cargo fmt --check -- all clean

Test plan

Second pass cleanup after PR #889. Removes all VIP, BGP, OSPF, BFD, and IPAM references from proto definitions, generated code, API types, CRD manifests, RBAC, Helm charts, kustomization, novactl CLI, operator code, controller logic, dataplane translator, Rust dataplane server, test files, sample YAML, and test fixtures. Key changes: - Proto: Remove VIPAssignment, VIPMode, BGPConfig, OSPFConfig, BFDConfig messages; remove VIP RPCs from config and dataplane protos; reserve field numbers for backward compatibility - API: Remove VIPRef from ProxyGatewaySpec - Controller: Remove VIP ref handling from ingress translator, remove --default-vip-ref flag from controller - Dataplane Go: Remove translateVIPs/translateVIPMode, UpsertVIP/DeleteVIP from translator and client - Dataplane Rust: Remove upsert_vip/delete_vip handlers, VIP apply logic, VIP status reporting, VIP module (garp, manager, routing) from server - CLI: Remove VIP/IPPool resource aliases, commands, and printer funcs - Tests: Remove all VIP-related test functions and test data - Config: Remove vipRef from all sample YAML, delete ProxyVIP CRD, clean RBAC and kustomization references

github-actions · 2026-03-07T13:57:31Z

📊 Benchmark Results

Symbol	Meaning
~	No significant change
+	Performance improved
-	Performance regressed

Copilot

Pull request overview

This PR completes the post-#889 cleanup by removing remaining VIP/BGP/OSPF/BFD/IPAM surface area across APIs, protos, controllers, dataplanes, CLI tooling, tests, and Helm/config manifests—aligning NovaEdge with the “pure L7 proxy” direction.

Changes:

Removes VIP-related proto messages/fields/RPCs (config, dataplane, federation) and updates generated stubs accordingly (with field-number reservations).
Removes vipRef from ProxyGateway API/CRD, controller translation paths (Ingress/Gateway API), dataplane translation, agent introspection/validation/persistence, and operator RBAC/webhooks.
Cleans up novactl commands/printers and test fixtures/scripts that referenced VIPs.

Reviewed changes

Copilot reviewed 84 out of 86 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
test/performance/k8s/perf-test-resources.yaml	Removes ProxyVIP perf fixture manifest (file deleted).
test/performance/k8s/l4-resources.yaml	Removes `vipRef` from perf L4 gateway sample.
test/functional/teardown.sh	Stops deleting ProxyVIP during functional teardown.
test/functional/setup.sh	Stops applying/verifying ProxyVIP; selects an agent pod directly.
test/functional/run-tests.sh	Removes VIP management test; selects agent pod without VIP election.
test/functional/02-proxygateway.yaml	Removes `vipRef` from functional gateway fixture.
test/functional/01-proxyvip.yaml	Removes functional ProxyVIP fixture (file deleted).
test/e2e/fixtures/compression-gateway.yaml	Removes `vipRef` from e2e gateway fixture.
internal/standalone/converter_test.go	Removes VIP mode parsing unit test.
internal/standalone/converter.go	Removes VIP conversion/parsing from standalone snapshot converter.
internal/proto/gen/config_grpc.pb.go	Removes ConfigService GetVIPs gRPC stubs (generated output updated).
internal/operator/webhook/federation_webhook.go	Removes ProxyVIP from allowed federation sync resource types.
internal/operator/controller/novaedgecluster_controller.go	Removes proxyvips RBAC rules from operator-managed ClusterRole.
internal/dataplane/translator_test.go	Removes VIP translation tests and VIP assertions in round-trip test.
internal/dataplane/translator.go	Removes VIP translation path and related logging/imports.
internal/dataplane/client_test.go	Removes fake server VIP RPCs and VIP client tests.
internal/dataplane/client.go	Removes VIP Upsert/Delete client methods.
internal/controller/snapshot/builder_test.go	Removes gateway `VIPRef` usage and VIP hashing/version tests.
internal/controller/snapshot/builder.go	Stops emitting/sorting VIP fields in snapshots/gateways.
internal/controller/proxypolicy_controller_test.go	Removes VIPRef from test ProxyGateway specs.
internal/controller/proxygateway_controller_test.go	Removes VIPRef from test ProxyGateway specs.
internal/controller/proxygateway_controller.go	Removes vipRef from reconcile log fields.
internal/controller/loadbalancerclass_test.go	Removes VIPRef from loadBalancerClass tests.
internal/controller/ingress_translator_extended_test.go	Removes VIPRef-related translator tests; updates constructor usage.
internal/controller/ingress_translator.go	Removes VIPRef annotation/defaulting and gateway VIPRef population.
internal/controller/ingress_controller.go	Drops DefaultVIPRef usage; status update is now explicitly a no-op.
internal/controller/gatewayapi_translator.go	Stops setting VIPRef on translated ProxyGateway; keeps arg for compatibility.
internal/controller/federation/integration_test.go	Removes VipRef from federation snapshot fixtures.
internal/agent/server/admin.go	Removes VIP counts from admin `/config` response.
internal/agent/introspection/server_test.go	Removes VIP-count and GetVIPs RPC tests.
internal/agent/introspection/server.go	Removes VipCount population and GetVIPs RPC implementation.
internal/agent/config/validation_test.go	Removes VIP validation tests and VIP fixtures from “valid snapshot”.
internal/agent/config/validation.go	Removes VIP assignment validation from snapshot validation.
internal/agent/config/persistence.go	Removes VIP count from persistence log metadata.
dataplane/novaedge-dataplane/src/vip/routing.rs	Removes Rust VIP routing module (file deleted).
dataplane/novaedge-dataplane/src/vip/mod.rs	Removes Rust VIP module exports (file deleted).
dataplane/novaedge-dataplane/src/vip/manager.rs	Removes Rust VIP manager implementation (file deleted).
dataplane/novaedge-dataplane/src/vip/garp.rs	Removes Rust GARP implementation (file deleted).
dataplane/novaedge-dataplane/src/server.rs	Removes VIP state/apply/RPC handlers; renames CIDR parsing helper.
dataplane/novaedge-dataplane/src/main.rs	Removes `mod vip;` from Rust dataplane crate.
config/webui/clusterrole.yaml	Removes proxyvips from WebUI ClusterRole.
config/samples/vault_gateway_sample.yaml	Removes `vipRef` from sample gateway YAML.
config/samples/tls_passthrough_sample.yaml	Removes `vipRef` from sample gateway YAML.
config/samples/tcp_proxy_sample.yaml	Removes `vipRef` from sample gateway YAML.
config/samples/proxygateway_test.yaml	Removes `vipRef` from sample gateway YAML.
config/samples/proxygateway_sample.yaml	Removes `vipRef` from sample gateway YAML.
config/samples/proxygateway_redirect_sample.yaml	Removes `vipRef` from sample gateway YAML.
config/samples/proxygateway_proxy_protocol_sample.yaml	Removes `vipRef` from sample gateway YAML.
config/samples/proxygateway_mtls_sample.yaml	Removes `vipRef` from sample gateway YAML.
config/samples/proxygateway_http3_sse_sample.yaml	Removes `vipRef` from sample gateway YAML.
config/samples/proxygateway_error_pages_sample.yaml	Removes `vipRef` from sample gateway YAML.
config/samples/proxygateway_compression_sample.yaml	Removes `vipRef` from sample gateway YAML.
config/samples/proxygateway_cache_sample.yaml	Removes `vipRef` from sample gateway YAML.
config/samples/http3_example.yaml	Removes `vipRef` from sample gateway YAML.
config/samples/certmanager_gateway_sample.yaml	Removes `vipRef` from sample gateway YAML.
config/rbac/role.yaml	Removes proxyippools/proxyvips from controller RBAC role.
config/default/kustomization.yaml	Removes ProxyVIP CRD from default kustomize resources.
config/crd/novaedge.io_proxygateways.yaml	Removes vipRef column/schema/required field from ProxyGateway CRD.
cmd/novaedge-controller/main.go	Removes `--default-vip-ref` flag and wiring.
cmd/novactl/pkg/printer/printer.go	Removes VIP/IPPool table printers and routing.
cmd/novactl/pkg/grpc/client.go	Removes VIP fields and GetVIPs client method.
cmd/novactl/pkg/client/client.go	Removes VIP/IPPool resource types and GVR mapping.
cmd/novactl/cmd/root.go	Updates CLI description to drop ProxyVIP mention.
cmd/novactl/cmd/metrics.go	Removes `metrics vips` subcommand.
cmd/novactl/cmd/get.go	Removes VIP/IPPool aliases and updates help text.
cmd/novactl/cmd/describe.go	Removes VIP/IPPool aliases.
cmd/novactl/cmd/delete.go	Removes VIP alias handling.
cmd/novactl/cmd/constants.go	Removes VIP/IPPool resource alias constants.
cmd/novactl/cmd/agent_query.go	Removes VIP subcommand output; still needs help-text cleanup.
charts/novaedge/values.yaml	Removes `controller.defaultVIPRef` values entry.
charts/novaedge/templates/webui-rbac.yaml	Removes proxyvips from chart WebUI RBAC.
charts/novaedge/templates/controller-rbac.yaml	Removes proxyippools/proxyvips from chart controller RBAC.
charts/novaedge/templates/controller-deployment.yaml	Removes `--default-vip-ref` arg injection.
charts/novaedge/crds/novaedge.io_proxygateways.yaml	Removes vipRef column/schema/required field from packaged CRD.
charts/novaedge/README.md	Removes ProxyVIP/ProxyIPPool from uninstall/CRD lists.
charts/novaedge-operator/crds/novaedge.io_proxyvips.yaml	Removes packaged ProxyVIP CRD (file deleted).
charts/novaedge-agent/templates/rbac.yaml	Removes proxyvips from agent RBAC resources list.
api/v1alpha1/proxygateway_types_test.go	Removes VIPRef from API type tests.
api/v1alpha1/proxygateway_types.go	Removes VIPRef from ProxyGatewaySpec and printcolumn.
api/proto/federation.proto	Removes vip_leader field, reserves tag number.
api/proto/dataplane/dataplane_grpc.pb.go	Removes VIP RPCs from generated dataplane gRPC stubs.
api/proto/dataplane.proto	Removes VIP RPCs/messages; reserves ApplyConfig field tag.
api/proto/config.proto	Removes VIP fields/messages/RPCs; reserves tag numbers in snapshot/introspection.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-07T14:03:00Z

+# Test 1: HTTP Routing
 # ============================================================
 echo "--- Test 2: HTTP Routing ---"
 PORT=$((PORT_BASE++))


The test header/comment numbering is inconsistent: the section is labeled "Test 1: HTTP Routing" but the echoed banner says "--- Test 2: HTTP Routing ---". This makes the output confusing when scanning logs; rename the echo (or the section header) to match.

Copilot · 2026-03-07T14:03:00Z

+    /// Parse an IP address with optional CIDR prefix (e.g. "10.0.0.1/32" -> (10.0.0.1, 32)).
    #[allow(clippy::result_large_err)]
-    fn parse_vip_address(addr: &str) -> Result<(std::net::IpAddr, u8), Status> {
+    fn parse_cidr_address(addr: &str) -> Result<(std::net::IpAddr, u8), Status> {
        let (ip_str, prefix_str) = addr.split_once('/').unwrap_or((addr, "32"));
        let ip: std::net::IpAddr = ip_str
            .parse()
-            .map_err(|e| Status::invalid_argument(format!("invalid VIP address '{addr}': {e}")))?;
+            .map_err(|e| Status::invalid_argument(format!("invalid address '{addr}': {e}")))?;
        let prefix: u8 = prefix_str
            .parse()
            .map_err(|e| Status::invalid_argument(format!("invalid prefix length: {e}")))?;
        Ok((ip, prefix))


parse_cidr_address always defaults a missing prefix to /32. For IPv6 addresses this should default to /128, otherwise a bare IPv6 address in flow filters becomes an unexpectedly broad network. Also consider validating prefix bounds per IP family (0..=32 for v4, 0..=128 for v6) and handling /0 explicitly—ip_in_cidr currently left-shifts by (32 - prefix) / (128 - prefix), which can overflow/panic when prefix is 0.

Copilot · 2026-03-07T14:03:00Z

 	cmd.AddCommand(newAgentConfigCommand())
 	cmd.AddCommand(newAgentBackendsCommand())
-	cmd.AddCommand(newAgentVIPsCommand())



The agent command help text still says it can query "active VIPs", but the VIP subcommand was removed in this change. Update the Long description so CLI help matches the actual supported subcommands.

Copilot · 2026-03-07T14:03:01Z

 	if len(agentConfig.Gateways) > 0 {
 		fmt.Printf("\nGateways:\n")
 		w := tabwriter.NewWriter(os.Stdout, 0, 8, 2, ' ', 0)
-		_, _ = fmt.Fprintln(w, "NAME\tNAMESPACE\tVIP\tLISTENERS")
+		_, _ = fmt.Fprintln(w, "NAME\tNAMESPACE\tLISTENERS")
 		for _, gw := range agentConfig.Gateways {


The agent config command help text mentions "VIP assignments", but VIPs were removed from the config/introspection surface in this PR. Please update the Long description for the command to avoid misleading users.

Copilot · 2026-03-07T14:03:01Z

-  // VIP assignments for this node
-  repeated VIPAssignment vip_assignments = 7;
+  // Field 7 reserved (was vip_assignments, removed in VIP cleanup)
+  reserved 7;


When removing protobuf fields, it's safer to reserve both the field number(s) and the removed field name(s) to prevent accidental reuse in future schema changes. Consider reserving the removed names (e.g., vip_assignments) alongside the numeric reserved entries.

Suggested change

reserved 7;

reserved 7;

reserved "vip_assignments";

Copilot · 2026-03-07T14:03:01Z

-  // vips are the virtual IP configurations.
-  repeated VIPConfig vips = 5;
+  // Field 5 reserved (was vips, removed in VIP cleanup).
+  reserved 5;


When removing protobuf fields, consider reserving the removed field name(s) in addition to the field number(s) to prevent accidental reuse later. Here, reserving the old name (previously vips) alongside reserved 5; would make the intent explicit.

Suggested change

reserved 5;

reserved 5;

reserved "vips";

Copilot · 2026-03-07T14:03:01Z

-  // Is this member the leader for L2 VIP coordination
-  bool vip_leader = 5;
+  // Field 5 reserved (was vip_leader, removed in VIP cleanup)
+  reserved 5;


When removing protobuf fields, consider reserving the removed field name(s) as well as the field number(s) to prevent accidental reuse. Reserving the old name (previously vip_leader) alongside reserved 5; would make the intent explicit.

Suggested change

reserved 5;

reserved 5;

reserved "vip_leader";

Copilot AI review requested due to automatic review settings March 7, 2026 13:57

piwi3910 merged commit ad3e499 into main Mar 7, 2026
2 of 3 checks passed

Copilot started reviewing on behalf of piwi3910 March 7, 2026 13:57 View session

Copilot AI reviewed Mar 7, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Remove remaining VIP/BGP/IPAM remnants#890

Remove remaining VIP/BGP/IPAM remnants#890
piwi3910 merged 1 commit into
mainfrom
fix/vip-remnants-cleanup

piwi3910 commented Mar 7, 2026

Uh oh!

Uh oh!

github-actions Bot commented Mar 7, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 7, 2026

Uh oh!

Copilot AI Mar 7, 2026

Uh oh!

Copilot AI Mar 7, 2026

Uh oh!

Copilot AI Mar 7, 2026

Uh oh!

Copilot AI Mar 7, 2026

Uh oh!

Copilot AI Mar 7, 2026

Uh oh!

Copilot AI Mar 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

piwi3910 commented Mar 7, 2026

Summary

Verification

Test plan

Uh oh!

Uh oh!

github-actions Bot commented Mar 7, 2026

📊 Benchmark Results

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants