diff --git a/pkg/internal/cyberark/dataupload/dataupload_test.go b/pkg/internal/cyberark/dataupload/dataupload_test.go index 2de68ad4..523c2c9b 100644 --- a/pkg/internal/cyberark/dataupload/dataupload_test.go +++ b/pkg/internal/cyberark/dataupload/dataupload_test.go @@ -144,10 +144,6 @@ func TestPostDataReadingsWithOptionsWithRealAPI(t *testing.T) { subdomain := os.Getenv("ARK_SUBDOMAIN") username := os.Getenv("ARK_USERNAME") secret := os.Getenv("ARK_SECRET") - serviceDiscoveryAPI := os.Getenv("ARK_DISCOVERY_API") - if serviceDiscoveryAPI == "" { - serviceDiscoveryAPI = servicediscovery.ProdDiscoveryEndpoint - } if platformDomain == "" || subdomain == "" || username == "" || secret == "" { t.Skip("Skipping because one of the following environment variables is unset or empty: ARK_PLATFORM_DOMAIN, ARK_SUBDOMAIN, ARK_USERNAME, ARK_SECRET") @@ -168,10 +164,7 @@ func TestPostDataReadingsWithOptionsWithRealAPI(t *testing.T) { httpClient := http_client.NewDefaultClient(version.UserAgent(), rootCAs) httpClient.Transport = transport.NewDebuggingRoundTripper(httpClient.Transport, transport.DebugByContext) - discoveryClient := servicediscovery.New( - servicediscovery.WithHTTPClient(httpClient), - servicediscovery.WithCustomEndpoint(serviceDiscoveryAPI), - ) + discoveryClient := servicediscovery.New(httpClient) identityAPI, err := discoveryClient.DiscoverIdentityAPIURL(ctx, subdomain) require.NoError(t, err) diff --git a/pkg/internal/cyberark/identity/cmd/testidentity/main.go b/pkg/internal/cyberark/identity/cmd/testidentity/main.go index 45e9b3bf..d0a68716 100644 --- a/pkg/internal/cyberark/identity/cmd/testidentity/main.go +++ b/pkg/internal/cyberark/identity/cmd/testidentity/main.go @@ -24,10 +24,9 @@ import ( // To test against a tenant on the integration platform, set: // ARK_DISCOVERY_API=https://platform-discovery.integration-cyberark.cloud/api/v2 const ( - subdomainFlag = "subdomain" - usernameFlag = "username" - passwordEnv = "ARK_SECRET" - serviceDiscoveryAPIEnv = "ARK_DISCOVERY_API" + subdomainFlag = "subdomain" + usernameFlag = "username" + passwordEnv = "ARK_SECRET" ) var ( @@ -49,19 +48,11 @@ func run(ctx context.Context) error { return fmt.Errorf("no password provided in %s", passwordEnv) } - serviceDiscoveryAPI := os.Getenv(serviceDiscoveryAPIEnv) - if serviceDiscoveryAPI == "" { - serviceDiscoveryAPI = servicediscovery.ProdDiscoveryEndpoint - } - var rootCAs *x509.CertPool httpClient := http_client.NewDefaultClient(version.UserAgent(), rootCAs) httpClient.Transport = transport.NewDebuggingRoundTripper(httpClient.Transport, transport.DebugByContext) - sdClient := servicediscovery.New( - servicediscovery.WithHTTPClient(httpClient), - servicediscovery.WithCustomEndpoint(serviceDiscoveryAPI), - ) + sdClient := servicediscovery.New(httpClient) identityAPI, err := sdClient.DiscoverIdentityAPIURL(ctx, subdomain) if err != nil { return fmt.Errorf("while performing service discovery: %s", err) diff --git a/pkg/internal/cyberark/servicediscovery/discovery.go b/pkg/internal/cyberark/servicediscovery/discovery.go index d99ddc79..a430d238 100644 --- a/pkg/internal/cyberark/servicediscovery/discovery.go +++ b/pkg/internal/cyberark/servicediscovery/discovery.go @@ -7,15 +7,13 @@ import ( "io" "net/http" "net/url" - "time" - - "k8s.io/client-go/transport" + "os" "github.com/jetstack/preflight/pkg/version" ) const ( - ProdDiscoveryEndpoint = "https://platform-discovery.cyberark.cloud/api/v2/" + ProdDiscoveryAPIBaseURL = "https://platform-discovery.cyberark.cloud/api/v2/" // identityServiceName is the name of the identity service we're looking for in responses from the Service Discovery API // We were told to use the identity_administration field, not the identity_user_portal field. @@ -30,40 +28,19 @@ const ( // users to fetch URLs for various APIs available in CyberArk. This client is specialised to // fetch only API endpoints, since only API endpoints are required by the Venafi Kubernetes Agent currently. type Client struct { - client *http.Client - endpoint string -} - -// ClientOpt allows configuration of a Client when using New -type ClientOpt func(*Client) - -// WithHTTPClient allows the user to specify a custom HTTP client for the discovery client -func WithHTTPClient(httpClient *http.Client) ClientOpt { - return func(c *Client) { - c.client = httpClient - } -} - -// WithCustomEndpoint sets the endpoint to a custom URL without checking that the URL is a CyberArk Service Discovery -// server. -func WithCustomEndpoint(endpoint string) ClientOpt { - return func(c *Client) { - c.endpoint = endpoint - } + client *http.Client + baseURL string } // New creates a new CyberArk Service Discovery client, configurable with ClientOpt -func New(clientOpts ...ClientOpt) *Client { - client := &Client{ - client: &http.Client{ - Timeout: 10 * time.Second, - Transport: transport.NewDebuggingRoundTripper(http.DefaultTransport, transport.DebugByContext), - }, - endpoint: ProdDiscoveryEndpoint, +func New(httpClient *http.Client) *Client { + baseURL := os.Getenv("ARK_DISCOVERY_API") + if baseURL == "" { + baseURL = ProdDiscoveryAPIBaseURL } - - for _, opt := range clientOpts { - opt(client) + client := &Client{ + client: httpClient, + baseURL: baseURL, } return client @@ -72,7 +49,7 @@ func New(clientOpts ...ClientOpt) *Client { // DiscoverIdentityAPIURL fetches from the service discovery service for a given subdomain // and parses the CyberArk Identity API URL. func (c *Client) DiscoverIdentityAPIURL(ctx context.Context, subdomain string) (string, error) { - endpoint, err := url.JoinPath(c.endpoint, "services", "subdomain", subdomain) + endpoint, err := url.JoinPath(c.baseURL, "services", "subdomain", subdomain) if err != nil { return "", fmt.Errorf("failed to build a valid URL for subdomain %s; possibly an invalid endpoint: %s", subdomain, err) } diff --git a/pkg/internal/cyberark/servicediscovery/discovery_test.go b/pkg/internal/cyberark/servicediscovery/discovery_test.go index 84bb9281..f18283e0 100644 --- a/pkg/internal/cyberark/servicediscovery/discovery_test.go +++ b/pkg/internal/cyberark/servicediscovery/discovery_test.go @@ -3,6 +3,11 @@ package servicediscovery import ( "fmt" "testing" + + "k8s.io/klog/v2" + "k8s.io/klog/v2/ktesting" + + _ "k8s.io/klog/v2/ktesting/init" ) func Test_DiscoverIdentityAPIURL(t *testing.T) { @@ -45,12 +50,12 @@ func Test_DiscoverIdentityAPIURL(t *testing.T) { for name, testSpec := range tests { t.Run(name, func(t *testing.T) { - ctx := t.Context() + logger := ktesting.NewLogger(t, ktesting.DefaultConfig) + ctx := klog.NewContext(t.Context(), logger) - ts := MockDiscoveryServer() - defer ts.Close() + httpClient := MockDiscoveryServer(t, mockIdentityAPIURL) - client := New(WithCustomEndpoint(ts.Server.URL)) + client := New(httpClient) apiURL, err := client.DiscoverIdentityAPIURL(ctx, testSpec.subdomain) if err != nil { diff --git a/pkg/internal/cyberark/servicediscovery/mock.go b/pkg/internal/cyberark/servicediscovery/mock.go index 928d8fc7..ceb0dce8 100644 --- a/pkg/internal/cyberark/servicediscovery/mock.go +++ b/pkg/internal/cyberark/servicediscovery/mock.go @@ -8,8 +8,11 @@ import ( "net/http" "net/http/httptest" "strings" + "testing" "text/template" + "k8s.io/client-go/transport" + "github.com/jetstack/preflight/pkg/version" _ "embed" @@ -19,50 +22,51 @@ const ( // MockDiscoverySubdomain is the subdomain for which the MockDiscoveryServer will return a success response MockDiscoverySubdomain = "venafi-test" - defaultIdentityAPIURL = "https://ajp5871.id.integration-cyberark.cloud" + mockIdentityAPIURL = "https://ajp5871.id.integration-cyberark.cloud" ) //go:embed testdata/discovery_success.json.template var discoverySuccessTemplate string type mockDiscoveryServer struct { - Server *httptest.Server - + t testing.TB successResponse string } -// MockDiscoveryServer returns a mocked discovery server with a default value for the Identity API. -// The returned server should be Closed by the caller after use. -func MockDiscoveryServer() *mockDiscoveryServer { - return MockDiscoveryServerWithCustomAPIURL(defaultIdentityAPIURL) -} - -func MockDiscoveryServerWithCustomAPIURL(apiURL string) *mockDiscoveryServer { +// MockDiscoveryServer starts a mocked CyberArk service discovery server and +// returns an HTTP client with the CA certs needed to connect to it. +// +// The URL of the mock server is set in the `ARK_DISCOVERY_API` environment +// variable, so any code using the `servicediscovery.Client` will use this mock +// server. +// +// The mock server will return a successful response when the subdomain is +// `MockDiscoverySubdomain`, and the identity API URL in that response will be +// `identityAPIURL`. +// Other subdomains, can be used to trigger various failure responses. +// The returned HTTP client has a transport which logs requests and responses +// depending on log level of the logger supplied in the context. +func MockDiscoveryServer(t testing.TB, identityAPIURL string) *http.Client { tmpl := template.Must(template.New("mockDiscoverySuccess").Parse(discoverySuccessTemplate)) - buf := &bytes.Buffer{} - - err := tmpl.Execute(buf, struct{ IdentityAPIURL string }{apiURL}) + err := tmpl.Execute(buf, struct{ IdentityAPIURL string }{identityAPIURL}) if err != nil { panic(err) } - mds := &mockDiscoveryServer{ + t: t, successResponse: buf.String(), } - - server := httptest.NewServer(mds) - - mds.Server = server - - return mds -} - -func (mds *mockDiscoveryServer) Close() { - mds.Server.Close() + server := httptest.NewTLSServer(mds) + t.Cleanup(server.Close) + t.Setenv("ARK_DISCOVERY_API", server.URL) + httpClient := server.Client() + httpClient.Transport = transport.NewDebuggingRoundTripper(httpClient.Transport, transport.DebugByContext) + return httpClient } func (mds *mockDiscoveryServer) ServeHTTP(w http.ResponseWriter, r *http.Request) { + mds.t.Log(r.Method, r.RequestURI) if r.Method != http.MethodGet { // This was observed by making a POST request to the integration environment // Normally, we'd expect 405 Method Not Allowed but we match the observed response here