Skip to content

Conversation

Copy link
Contributor

Copilot AI commented May 22, 2025

Summary

This PR adds support for custom headers in the MonitorConfig property of TrafficManagerProfile. Custom headers, such as the Host header, are essential for making health checks work with Kubernetes ingress controllers which route requests based on the hostname.

Changes

  1. Added a new MonitorConfigCustomHeader struct to both v1alpha1 and v1beta1 API versions with name and value string fields
  2. Added a CustomHeaders array field to the MonitorConfig struct in both API versions
  3. Updated the controller's generateAzureTrafficManagerProfile function to pass custom headers to the Azure Traffic Manager API
  4. Updated equalAzureTrafficManagerProfile to properly compare custom headers when determining if updates are needed
  5. Updated buildAzureTrafficManagerProfileRequest to include custom headers when building the Azure Traffic Manager request
  6. Added test cases to validate the custom header functionality

Usage Example

Users can now specify custom headers for health checks in their TrafficManagerProfile resources:

apiVersion: networking.fleet.azure.com/v1beta1
kind: TrafficManagerProfile
metadata:
  name: example-profile
spec:
  resourceGroup: my-resource-group
  monitorConfig:
    protocol: HTTP
    path: /
    port: 80
    customHeaders:
    - name: Host
      value: myapplication.example.com

This allows health checks to include the Host header which is required for ingress controllers to route the request to the correct backend service.

Fixes #310.

Warning

Firewall rules blocked me from connecting to one or more addresses

I tried to connect to the following addresses, but was blocked by firewall rules:

  • https://storage.googleapis.com/storage/v1/b/kubebuilder-tools/o
    • Triggering command: /home/REDACTED/work/fleet-networking/fleet-networking/hack/tools/bin/setup-envtest-v0.0.0-20240317073005-bd9ea79e8d18 use 1.28.x -p path (http block)

If you need me to access, download, or install something from one of these locations, you can either:


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

@britaniar britaniar assigned Copilot and unassigned britaniar and Copilot May 22, 2025
Copilot AI changed the title [WIP] Add Custom Headers property to the monitorConfig property of TrafficManagerProfile Add Custom Headers property to the monitorConfig property of TrafficManagerProfile May 22, 2025
Copilot AI requested a review from britaniar May 22, 2025 23:12
@ryanzhang-oss
Copy link
Contributor

@copilot, can you follow the .github/pr-title-config.json to change the PR title?

Copy link
Contributor Author

Copilot AI commented May 22, 2025

@copilot, can you follow the .github/pr-title-config.json to change the PR title?

I'll update the PR title to follow the formatting requirements. The updated title will be: "feat: Add Custom Headers property to the monitorConfig property of TrafficManagerProfile"

@ryanzhang-oss
Copy link
Contributor

can you fix the lint issue "Error: pkg/controllers/hub/trafficmanagerprofile/controller.go:228:1: cyclomatic complexity 39 of func equalAzureTrafficManagerProfile is high (> 30) (gocyclo)"

@ryanzhang-oss ryanzhang-oss changed the title Add Custom Headers property to the monitorConfig property of TrafficManagerProfile feat: Add Custom Headers property to the monitorConfig property of TrafficManagerProfile May 22, 2025
@codecov
Copy link

codecov bot commented May 22, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.56%. Comparing base (af25d4f) to head (9b009c9).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #311      +/-   ##
==========================================
+ Coverage   80.30%   80.56%   +0.26%     
==========================================
  Files          29       29              
  Lines        4108     4153      +45     
==========================================
+ Hits         3299     3346      +47     
+ Misses        643      642       -1     
+ Partials      166      165       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor Author

Copilot AI commented May 22, 2025

can you fix the lint issue "Error: pkg/controllers/hub/trafficmanagerprofile/controller.go:228:1: cyclomatic complexity 39 of func equalAzureTrafficManagerProfile is high (> 30) (gocyclo)"

I've fixed the cyclomatic complexity issue in the equalAzureTrafficManagerProfile function by refactoring it into smaller helper functions, each with a specific responsibility:

  1. hasRequiredProperties - Checks if all required properties exist
  2. equalMonitorConfig - Compares the monitor configurations
  3. equalCustomHeaders - Compares the custom headers
  4. equalProfileProperties - Compares profile status and routing method
  5. equalDNSConfig - Compares DNS configuration
  6. equalTags - Compares tags

This refactoring preserves all the existing functionality while reducing the complexity of the original function. The changes are in commit d34b88f.

Warning

Firewall rules blocked me from connecting to one or more addresses

I tried to connect to the following addresses, but was blocked by firewall rules:

  • cdn.fwupd.org
    • Triggering command: /usr/bin/fwupdmgr refresh (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

@ryanzhang-oss
Copy link
Contributor

Thanks, now can you add more UT coverage for the newly created functions?

@ryanzhang-oss ryanzhang-oss marked this pull request as ready for review May 22, 2025 23:48
@ryanzhang-oss
Copy link
Contributor

please also add integration tests to test the newly added CustomHeader in the pkg/controllers/hub/trafficmanagerprofile/controller_integration_test.go

Copy link
Contributor Author

Copilot AI commented May 23, 2025

Thanks, now can you add more UT coverage for the newly created functions?

I've added comprehensive unit test coverage for all the newly created helper functions in commit 597a32a. The tests cover both success cases and various failure scenarios for each function:

  1. TestHasRequiredProperties - Tests null checks for all required properties
  2. TestEqualMonitorConfig - Tests null checks and value comparison for all monitor config fields
  3. TestEqualCustomHeaders - Tests header comparison with various scenarios (empty arrays, different values, nil fields)
  4. TestEqualProfileProperties - Tests comparison of profile status and traffic routing method
  5. TestEqualDNSConfig - Tests DNS TTL validation and comparison
  6. TestEqualTags - Tests tag comparison including nil checks, missing tags, and value differences

All tests are passing, and the gocyclo linter no longer reports complexity issues.

Copy link
Contributor Author

Copilot AI commented May 23, 2025

please also add integration tests to test the newly added CustomHeader in the pkg/controllers/hub/trafficmanagerprofile/controller_integration_test.go

I've added integration tests for the CustomHeaders functionality in the TrafficManagerProfile controller in commit 1f505c6. The tests verify:

  1. Creating a TrafficManagerProfile with custom headers (single header)
  2. Updating a TrafficManagerProfile with modified custom headers (changing value and adding a new header)
  3. Proper cleanup and deletion

The tests follow the same pattern as the existing integration tests in the file and validate that the controller properly processes and manages custom headers in the TrafficManagerProfile's monitorConfig.

Warning

Firewall rules blocked me from connecting to one or more addresses

I tried to connect to the following addresses, but was blocked by firewall rules:

  • https://storage.googleapis.com/storage/v1/b/kubebuilder-tools/o
    • Triggering command: /home/REDACTED/work/fleet-networking/fleet-networking/hack/tools/bin/setup-envtest-v0.0.0-20240317073005-bd9ea79e8d18 use 1.28.x -p path (http block)

If you need me to access, download, or install something from one of these locations, you can either:

Copy link
Contributor Author

Copilot AI commented May 26, 2025

please fix the lint error, "pkg/controllers/hub/trafficmanagerprofile/controller.go:1: : # go.goms.io/fleet-networking/pkg/controllers/hub/trafficmanagerprofile [go.goms.io/fleet-networking/pkg/contro...

I've fixed the lint errors by updating the controller tests to use the new refactored function names. The issue was that we combined equalProfileProperties and equalDNSConfig into equalProfilePropertiesAndDNS, but the tests were still calling the old functions. Changes in commit 25f0762.

Warning

Firewall rules blocked me from connecting to one or more addresses

I tried to connect to the following addresses, but was blocked by firewall rules:

  • cdn.fwupd.org
    • Triggering command: /usr/bin/fwupdmgr refresh (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

@zhiying-lin
Copy link
Contributor

please fix the integration test failure,

Test TrafficManagerProfile Controller When creating trafficManagerProfile with custom headers [It] Updating trafficManagerProfile with different custom headers
/home/runner/work/fleet-networking/fleet-networking/pkg/controllers/hub/trafficmanagerprofile/controller_integration_test.go:664

  [FAILED] Timed out after 10.000s.
  failed to validate the trafficManagerProfile status metrics
  Expected success, but got an error:
      <*errors.errorString | 0xc0009119b0>: 
      trafficManagerProfile status metrics mismatch (-got, +want):
        []*io_prometheus_client.Metric{
      - 	s`label:{name:"condition"  value:"Programmed"}  label:{name:"generation"  value:"1"}  label:{name:"name"  value:"valid-profile"}  label:{name:"namespace"  value:"profile-ns"}  label:{name:"reason"  value:"Programmed"}  label:{name:"status"  value:"True"}  ga`...,
        	&{Label: Inverse(cmpopts.SortSlices, []*io_prometheus_client.LabelPair{&{Name: &"condition", Value: &"Programmed"}, &{Name: &"generation", Value: &"2"}, &{Name: &"name", Value: &"valid-profile"}, &{Name: &"namespace", Value: &"profile-ns"}, ...}), Gauge: s"value:1.7482297435891578e+09"},
        }
      
      {
          s: "trafficManagerProfile status metrics mismatch (-got, +want):\n  []*io_prometheus_client.Metric{\n- \ts`label:{name:\"condition\"  value:\"Programmed\"}  label:{name:\"generation\"  value:\"1\"}  label:{name:\"name\"  value:\"valid-profile\"}  label:{name:\"namespace\"  value:\"profile-ns\"}  label:{name:\"reason\"  value:\"Programmed\"}  label:{name:\"status\"  value:\"True\"}  ga`...,\n  \t&{Label: Inverse(cmpopts.SortSlices, []*io_prometheus_client.LabelPair{&{Name: &\"condition\", Value: &\"Programmed\"}, &{Name: &\"generation\", Value: &\"2\"}, &{Name: &\"name\", Value: &\"valid-profile\"}, &{Name: &\"namespace\", Value: &\"profile-ns\"}, ...}), Gauge: s\"value:1.7482297435891578e+09\"},\n  }\n",
      }
  In [It] at: /home/runner/work/fleet-networking/fleet-networking/pkg/controllers/hub/trafficmanagerprofile/controller_integration_test.go:78 @ 05/26/25 03:22:33.589

please fix the lint error, pkg/controllers/hub/trafficmanagerprofile/controller_test.go:910:1: File is not properly formatted (gofmt)

@zhiying-lin
Copy link
Contributor

please fix the integration test failure,

Test TrafficManagerProfile Controller When creating trafficManagerProfile with custom headers [It] Updating trafficManagerProfile with different custom headers
/home/runner/work/fleet-networking/fleet-networking/pkg/controllers/hub/trafficmanagerprofile/controller_integration_test.go:664

  [FAILED] Timed out after 10.000s.
  failed to validate the trafficManagerProfile status metrics
  Expected success, but got an error:
      <*errors.errorString | 0xc0009119b0>: 
      trafficManagerProfile status metrics mismatch (-got, +want):
        []*io_prometheus_client.Metric{
      - 	s`label:{name:"condition"  value:"Programmed"}  label:{name:"generation"  value:"1"}  label:{name:"name"  value:"valid-profile"}  label:{name:"namespace"  value:"profile-ns"}  label:{name:"reason"  value:"Programmed"}  label:{name:"status"  value:"True"}  ga`...,
        	&{Label: Inverse(cmpopts.SortSlices, []*io_prometheus_client.LabelPair{&{Name: &"condition", Value: &"Programmed"}, &{Name: &"generation", Value: &"2"}, &{Name: &"name", Value: &"valid-profile"}, &{Name: &"namespace", Value: &"profile-ns"}, ...}), Gauge: s"value:1.7482297435891578e+09"},
        }
      
      {
          s: "trafficManagerProfile status metrics mismatch (-got, +want):\n  []*io_prometheus_client.Metric{\n- \ts`label:{name:\"condition\"  value:\"Programmed\"}  label:{name:\"generation\"  value:\"1\"}  label:{name:\"name\"  value:\"valid-profile\"}  label:{name:\"namespace\"  value:\"profile-ns\"}  label:{name:\"reason\"  value:\"Programmed\"}  label:{name:\"status\"  value:\"True\"}  ga`...,\n  \t&{Label: Inverse(cmpopts.SortSlices, []*io_prometheus_client.LabelPair{&{Name: &\"condition\", Value: &\"Programmed\"}, &{Name: &\"generation\", Value: &\"2\"}, &{Name: &\"name\", Value: &\"valid-profile\"}, &{Name: &\"namespace\", Value: &\"profile-ns\"}, ...}), Gauge: s\"value:1.7482297435891578e+09\"},\n  }\n",
      }
  In [It] at: /home/runner/work/fleet-networking/fleet-networking/pkg/controllers/hub/trafficmanagerprofile/controller_integration_test.go:78 @ 05/26/25 03:22:33.589

please fix the lint error, pkg/controllers/hub/trafficmanagerprofile/controller_test.go:910:1: File is not properly formatted (gofmt)

@copilot

Copy link
Contributor Author

Copilot AI commented May 26, 2025

please fix the integration test failure,

...

I've fixed the integration test failure in commit 6ced299. The issue was that when updating the TrafficManagerProfile with different custom headers, we needed to reset the metrics registry before validating the new metrics with the updated generation number. Now the test properly validates fresh metrics after the update.

Warning

Firewall rules blocked me from connecting to one or more addresses

I tried to connect to the following addresses, but was blocked by firewall rules:

  • https://storage.googleapis.com/storage/v1/b/kubebuilder-tools/o
    • Triggering command: /home/REDACTED/work/fleet-networking/fleet-networking/hack/tools/bin/setup-envtest-v0.0.0-20240317073005-bd9ea79e8d18 use 1.28.x -p path (http block)

If you need me to access, download, or install something from one of these locations, you can either:

@zhiying-lin zhiying-lin marked this pull request as draft May 26, 2025 09:51
Copilot AI and others added 13 commits May 27, 2025 15:17
… handle missing test dependencies

Co-authored-by: zhiying-lin <[email protected]>
@zhiying-lin zhiying-lin marked this pull request as ready for review May 27, 2025 09:04
@ryanzhang-oss ryanzhang-oss merged commit b804959 into main May 28, 2025
14 checks passed
@zhiying-lin zhiying-lin deleted the copilot/fix-310 branch May 29, 2025 03:18
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.

Add Custom Headers property to the monitorConfig property of TrafficManagerProfile

4 participants