[VC-43403] CyberArk: enhance service discovery to return multiple APIs

pkg/internal/cyberark/dataupload/dataupload.go

@@ -24,7 +24,7 @@ const (
 	// apiPathSnapshotLinks is the URL path of the snapshot-links endpoint of the inventory API.
 	// This endpoint returns an AWS presigned URL.
 	// TODO(wallrj): Link to CyberArk API documentation when it is published.
-	apiPathSnapshotLinks = "/api/ingestions/kubernetes/snapshot-links"
+	apiPathSnapshotLinks = "/ingestions/kubernetes/snapshot-links"
 )
 
 type CyberArkClient struct {

pkg/internal/cyberark/dataupload/dataupload_test.go

@@ -124,7 +124,6 @@ func TestCyberArkClient_PostDataReadingsWithOptions(t *testing.T) {
 // TestPostDataReadingsWithOptionsWithRealAPI demonstrates that the dataupload code works with the real inventory API.
 // An API token is obtained by authenticating with the ARK_USERNAME and ARK_SECRET from the environment.
 // ARK_SUBDOMAIN should be your tenant subdomain.
-// ARK_PLATFORM_DOMAIN should be either integration-cyberark.cloud or cyberark.cloud
 //
 // To test against a tenant on the integration platform, also set:
 // ARK_DISCOVERY_API=https://platform-discovery.integration-cyberark.cloud/api/v2
@@ -134,36 +133,32 @@ func TestCyberArkClient_PostDataReadingsWithOptions(t *testing.T) {
 //	go test ./pkg/internal/cyberark/dataupload/... \
 //	  -v -count 1 -run TestPostDataReadingsWithOptionsWithRealAPI -args -testing.v 6
 func TestPostDataReadingsWithOptionsWithRealAPI(t *testing.T) {
-	platformDomain := os.Getenv("ARK_PLATFORM_DOMAIN")
 	subdomain := os.Getenv("ARK_SUBDOMAIN")
 	username := os.Getenv("ARK_USERNAME")
 	secret := os.Getenv("ARK_SECRET")
 
-	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")
+	if subdomain == "" || username == "" || secret == "" {
+		t.Skip("Skipping because one of the following environment variables is unset or empty: ARK_SUBDOMAIN, ARK_USERNAME, ARK_SECRET")
 		return
 	}
 
 	logger := ktesting.NewLogger(t, ktesting.DefaultConfig)
 	ctx := klog.NewContext(t.Context(), logger)
 
-	// TODO(wallrj): get this from the servicediscovery API instead.
-	inventoryAPI := fmt.Sprintf("https://%s.inventory.%s", subdomain, platformDomain)
-
 	var rootCAs *x509.CertPool
 	httpClient := http_client.NewDefaultClient(version.UserAgent(), rootCAs)
 	httpClient.Transport = transport.NewDebuggingRoundTripper(httpClient.Transport, transport.DebugByContext)
 
 	discoveryClient := servicediscovery.New(httpClient)
 
-	identityAPI, err := discoveryClient.DiscoverIdentityAPIURL(ctx, subdomain)
+	services, err := discoveryClient.DiscoverServices(ctx, subdomain)
 	require.NoError(t, err)
 
-	identityClient := identity.New(httpClient, identityAPI, subdomain)
+	identityClient := identity.New(httpClient, services.Identity.API, subdomain)
 	err = identityClient.LoginUsernamePassword(ctx, username, []byte(secret))
 	require.NoError(t, err)
 
-	cyberArkClient := dataupload.New(httpClient, inventoryAPI, identityClient.AuthenticateRequest)
+	cyberArkClient := dataupload.New(httpClient, services.DiscoveryContext.API, identityClient.AuthenticateRequest)
 	err = cyberArkClient.PostDataReadingsWithOptions(ctx, api.DataReadingsPost{}, dataupload.Options{
 		ClusterName: "bb068932-c80d-460d-88df-34bc7f3f3297",
 	})

pkg/internal/cyberark/identity/cmd/testidentity/main.go

@@ -53,12 +53,12 @@ func run(ctx context.Context) error {
 	httpClient.Transport = transport.NewDebuggingRoundTripper(httpClient.Transport, transport.DebugByContext)
 
 	sdClient := servicediscovery.New(httpClient)
-	identityAPI, err := sdClient.DiscoverIdentityAPIURL(ctx, subdomain)
+	services, err := sdClient.DiscoverServices(ctx, subdomain)
 	if err != nil {
 		return fmt.Errorf("while performing service discovery: %s", err)
 	}
 
-	client := identity.New(httpClient, identityAPI, subdomain)
+	client := identity.New(httpClient, services.Identity.API, subdomain)
 
 	err = client.LoginUsernamePassword(ctx, username, []byte(password))
 	if err != nil {

pkg/internal/cyberark/servicediscovery/discovery.go

@@ -13,6 +13,7 @@ import (
 )
 
 const (
+	// ProdDiscoveryAPIBaseURL is the base URL for the production CyberArk Service Discovery API
 	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
@@ -32,7 +33,9 @@ type Client struct {
 	baseURL string
 }
 
-// New creates a new CyberArk Service Discovery client, configurable with ClientOpt
+// New creates a new CyberArk Service Discovery client. If the ARK_DISCOVERY_API
+// environment variable is set, it is used as the base URL for the service
+// discovery API. Otherwise, the production URL is used.
 func New(httpClient *http.Client) *Client {
 	baseURL := os.Getenv("ARK_DISCOVERY_API")
 	if baseURL == "" {
@@ -46,25 +49,43 @@ func New(httpClient *http.Client) *Client {
 	return 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) {
+// ServiceEndpoint represents a single service endpoint returned by the CyberArk
+// Service Discovery API. The JSON field names here must match the field names
+// returned by the Service Discovery API. Currently, we only care about the
+// "api" field. Other fields are intentionally ignored here.
+type ServiceEndpoint struct {
+	API string `json:"api"`
+}
+
+// Services represents the relevant services returned by the CyberArk Service
+// Discovery API for a given subdomain. Currently, we only care about the
+// Identity API and the Discovery Context API. Other services are intentionally
+// ignored here. The JSON field names here must match the field names returned
+// by the Service Discovery API.
+type Services struct {
+	Identity         ServiceEndpoint `json:"identity_administration"`
+	DiscoveryContext ServiceEndpoint `json:"discoverycontext"`
+}
+
+// DiscoverServices fetches from the service discovery service for a given subdomain
+// and parses the CyberArk Identity API URL and Inventory API URL.
+func (c *Client) DiscoverServices(ctx context.Context, subdomain string) (*Services, error) {
 	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)
+		return nil, fmt.Errorf("failed to build a valid URL for subdomain %s; possibly an invalid endpoint: %s", subdomain, err)
 	}
 
 	request, err := http.NewRequestWithContext(ctx, http.MethodGet, endpoint, nil)
 	if err != nil {
-		return "", fmt.Errorf("failed to initialise request to %s: %s", endpoint, err)
+		return nil, fmt.Errorf("failed to initialise request to %s: %s", endpoint, err)
 	}
 
 	request.Header.Set("Accept", "application/json")
 	version.SetUserAgent(request)
 
 	resp, err := c.client.Do(request)
 	if err != nil {
-		return "", fmt.Errorf("failed to perform HTTP request: %s", err)
+		return nil, fmt.Errorf("failed to perform HTTP request: %s", err)
 	}
 
 	defer resp.Body.Close()
@@ -73,32 +94,26 @@ func (c *Client) DiscoverIdentityAPIURL(ctx context.Context, subdomain string) (
 		// a 404 error is returned with an empty JSON body "{}" if the subdomain is unknown; at the time of writing, we haven't observed
 		// any other errors and so we can't special case them
 		if resp.StatusCode == http.StatusNotFound {
-			return "", fmt.Errorf("got an HTTP 404 response from service discovery; maybe the subdomain %q is incorrect or does not exist?", subdomain)
+			return nil, fmt.Errorf("got an HTTP 404 response from service discovery; maybe the subdomain %q is incorrect or does not exist?", subdomain)
 		}
 
-		return "", fmt.Errorf("got unexpected status code %s from request to service discovery API", resp.Status)
-	}
-
-	type ServiceEndpoint struct {
-		API string `json:"api"`
-		// NB: other fields are intentionally ignored here; we only care about the API URL
+		return nil, fmt.Errorf("got unexpected status code %s from request to service discovery API", resp.Status)
 	}
 
-	decodedResponse := make(map[string]ServiceEndpoint)
+	var services Services
 
-	err = json.NewDecoder(io.LimitReader(resp.Body, maxDiscoverBodySize)).Decode(&decodedResponse)
+	err = json.NewDecoder(io.LimitReader(resp.Body, maxDiscoverBodySize)).Decode(&services)
 	if err != nil {
 		if err == io.ErrUnexpectedEOF {
-			return "", fmt.Errorf("rejecting JSON response from server as it was too large or was truncated")
+			return nil, fmt.Errorf("rejecting JSON response from server as it was too large or was truncated")
 		}
 
-		return "", fmt.Errorf("failed to parse JSON from otherwise successful request to service discovery endpoint: %s", err)
+		return nil, fmt.Errorf("failed to parse JSON from otherwise successful request to service discovery endpoint: %s", err)
 	}
 
-	identityService, ok := decodedResponse[identityServiceName]
-	if !ok {
-		return "", fmt.Errorf("didn't find %s in service discovery response, which may indicate a suspended tenant; unable to detect CyberArk Identity API URL", identityServiceName)
+	if services.Identity.API == "" {
+		return nil, fmt.Errorf("didn't find %s in service discovery response, which may indicate a suspended tenant; unable to detect CyberArk Identity API URL", identityServiceName)
 	}
 
-	return identityService.API, nil
+	return &services, nil
 }

pkg/internal/cyberark/servicediscovery/discovery_test.go

@@ -4,6 +4,8 @@ import (
 	"fmt"
 	"testing"
 
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	"k8s.io/klog/v2"
 	"k8s.io/klog/v2/ktesting"
 
@@ -53,21 +55,26 @@ func Test_DiscoverIdentityAPIURL(t *testing.T) {
 			logger := ktesting.NewLogger(t, ktesting.DefaultConfig)
 			ctx := klog.NewContext(t.Context(), logger)
 
-			httpClient := MockDiscoveryServer(t, mockIdentityAPIURL)
+			httpClient := MockDiscoveryServer(t, Services{
+				Identity: ServiceEndpoint{
+					API: mockIdentityAPIURL,
+				},
+				DiscoveryContext: ServiceEndpoint{
+					API: mockDiscoveryContextAPIURL,
+				},
+			})
 
 			client := New(httpClient)
 
-			apiURL, err := client.DiscoverIdentityAPIURL(ctx, testSpec.subdomain)
-			if err != nil {
-				if err.Error() != testSpec.expectedError.Error() {
-					t.Errorf("expectedError=%v\nobservedError=%v", testSpec.expectedError, err)
-				}
+			services, err := client.DiscoverServices(ctx, testSpec.subdomain)
+			if testSpec.expectedError != nil {
+				assert.EqualError(t, err, testSpec.expectedError.Error())
+				assert.Nil(t, services)
+				return
 			}
-
-			// NB: we don't exit here because we also want to check the API URL is empty in the event of an error
-
-			if apiURL != testSpec.expectedURL {
-				t.Errorf("expected API URL=%s\nobserved API URL=%s", testSpec.expectedURL, apiURL)
+			require.NoError(t, err)
+			if services.Identity.API != testSpec.expectedURL {
+				t.Errorf("expected API URL=%s\nobserved API URL=%s", testSpec.expectedURL, services.Identity.API)
 			}
 		})
 	}

pkg/internal/cyberark/servicediscovery/mock.go

@@ -22,7 +22,8 @@ const (
 	// MockDiscoverySubdomain is the subdomain for which the MockDiscoveryServer will return a success response
 	MockDiscoverySubdomain = "venafi-test"
 
-	mockIdentityAPIURL = "https://ajp5871.id.integration-cyberark.cloud"
+	mockIdentityAPIURL         = "https://ajp5871.id.integration-cyberark.cloud"
+	mockDiscoveryContextAPIURL = "https://venafi-test.inventory.integration-cyberark.cloud/api"
 )
 
 //go:embed testdata/discovery_success.json.template
@@ -41,15 +42,16 @@ type mockDiscoveryServer struct {
 // 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`.
+// `MockDiscoverySubdomain`, and the API URLs in the response will match those
+// supplied in `services`.
 // 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 {
+func MockDiscoveryServer(t *testing.T, services Services) *http.Client {
 	tmpl := template.Must(template.New("mockDiscoverySuccess").Parse(discoverySuccessTemplate))
 	buf := &bytes.Buffer{}
-	err := tmpl.Execute(buf, struct{ IdentityAPIURL string }{identityAPIURL})
+	err := tmpl.Execute(buf, services)
 	if err != nil {
 		panic(err)
 	}

pkg/internal/cyberark/servicediscovery/testdata/README.md

@@ -1,3 +1,13 @@
 # Test data for CyberArk Discovery
 
-All data in this folder is derived from an unauthenticated endpoint accessible from the public internet.
+All data in this folder is derived from an unauthenticated endpoint accessible from the public Internet.
+
+To get the original data:
+
+```bash
+curl -fsSL "${ARK_DISCOVERY_API}/services/subdomain/${ARK_SUBDOMAIN}" | jq
+```
+
+Then replace `identity_administration.api` with `{{ .Identity.API }}` and
+`discoverycontext.api` with `{{ .DiscoveryContext.API }}`. Those Go template
+fields will be substituted in the tests.