Skip to content

Conversation

wallrj-cyberark
Copy link
Member

@wallrj-cyberark wallrj-cyberark commented Aug 27, 2025

  • Removed WithCustomEndpoint and WithHTTPClient options from the servicediscovery.Client.
  • Updated New function to directly use http.Client and fetch the base URL from the ARK_DISCOVERY_API environment variable.
  • Replaced ProdDiscoveryEndpoint constant with ProdDiscoveryAPIBaseURL for clarity.
  • Refactored tests to use testing.T cleanup and environment variable setup for mock servers.
  • Simplified test setup by removing redundant mock server initialization steps.

Follow up PRs

Testing

Example of the extra logging from the tests, which helps diagnose test failures and helps to understand what API requests the client is making:

$ go test ./pkg/internal/cyberark/servicediscovery/... -v -args -testing.v 6
=== RUN   Test_DiscoverIdentityAPIURL
=== RUN   Test_DiscoverIdentityAPIURL/successful_request
    mock.go:69: GET /services/subdomain/venafi-test
    round_trippers.go:632: I0827 17:33:38.601555] Response verb="GET" url="https://127.0.0.1:36789/services/subdomain/venafi-test" status="200 OK" milliseconds=2
=== RUN   Test_DiscoverIdentityAPIURL/subdomain_not_found
    mock.go:69: GET /services/subdomain/something-random
    round_trippers.go:632: I0827 17:33:38.605067] Response verb="GET" url="https://127.0.0.1:35145/services/subdomain/something-random" status="404 Not Found" milliseconds=3
=== RUN   Test_DiscoverIdentityAPIURL/no_identity_service_in_response
    mock.go:69: GET /services/subdomain/no-identity
    round_trippers.go:632: I0827 17:33:38.607924] Response verb="GET" url="https://127.0.0.1:40485/services/subdomain/no-identity" status="200 OK" milliseconds=1
=== RUN   Test_DiscoverIdentityAPIURL/unexpected_HTTP_response
    mock.go:69: GET /services/subdomain/bad-request
    round_trippers.go:632: I0827 17:33:38.610534] Response verb="GET" url="https://127.0.0.1:36017/services/subdomain/bad-request" status="400 Bad Request" milliseconds=2
=== RUN   Test_DiscoverIdentityAPIURL/response_JSON_too_long
    mock.go:69: GET /services/subdomain/json-too-long
    round_trippers.go:632: I0827 17:33:38.696813] Response verb="GET" url="https://127.0.0.1:36707/services/subdomain/json-too-long" status="200 OK" milliseconds=85
=== RUN   Test_DiscoverIdentityAPIURL/response_JSON_invalid
    mock.go:69: GET /services/subdomain/json-invalid
    round_trippers.go:632: I0827 17:33:38.708833] Response verb="GET" url="https://127.0.0.1:32793/services/subdomain/json-invalid" status="200 OK" milliseconds=2
--- PASS: Test_DiscoverIdentityAPIURL (0.11s)
    --- PASS: Test_DiscoverIdentityAPIURL/successful_request (0.00s)
    --- PASS: Test_DiscoverIdentityAPIURL/subdomain_not_found (0.00s)
    --- PASS: Test_DiscoverIdentityAPIURL/no_identity_service_in_response (0.00s)
    --- PASS: Test_DiscoverIdentityAPIURL/unexpected_HTTP_response (0.00s)
    --- PASS: Test_DiscoverIdentityAPIURL/response_JSON_too_long (0.09s)
    --- PASS: Test_DiscoverIdentityAPIURL/response_JSON_invalid (0.00s)
PASS
ok      github.com/jetstack/preflight/pkg/internal/cyberark/servicediscovery    0.118s

@wallrj-cyberark wallrj-cyberark force-pushed the VC-43403-servicediscovery-client branch from 5ebbb26 to 76649af Compare August 27, 2025 13:14
@wallrj-cyberark wallrj-cyberark force-pushed the VC-43403-servicediscovery-client branch from 76649af to 74ab7bc Compare August 27, 2025 15:58
Base automatically changed from VC-43403-identity-client to master August 27, 2025 16:09
@wallrj-cyberark wallrj-cyberark force-pushed the VC-43403-servicediscovery-client branch 3 times, most recently from 9d9ff30 to 25eccd6 Compare August 27, 2025 16:30
… unused options

- Removed `WithHTTPClient` and `WithCustomEndpoint` options from the `servicediscovery.Client`.
- Simplified `New` function to directly use `http.Client` and environment variable for base URL.
- Updated tests to use `MockDiscoveryServer` with `testing.TB` for better integration.
- Replaced hardcoded `ProdDiscoveryEndpoint` with `ProdDiscoveryAPIBaseURL` for consistency.
- Adjusted dependent code to align with the new client initialization approach.
@wallrj-cyberark wallrj-cyberark force-pushed the VC-43403-servicediscovery-client branch from 25eccd6 to e61ad7a Compare August 27, 2025 16:31
@wallrj-cyberark wallrj-cyberark marked this pull request as ready for review August 27, 2025 16:32
@wallrj-cyberark wallrj-cyberark merged commit 9b720ac into master Aug 28, 2025
2 checks passed
@wallrj-cyberark wallrj-cyberark deleted the VC-43403-servicediscovery-client branch August 28, 2025 08:20
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.

2 participants