-
Notifications
You must be signed in to change notification settings - Fork 25
[VC-43753] Update the dataupload package to be compatible with the inventory API #681
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
ed75fe8
to
07889c7
Compare
07889c7
to
f2ae463
Compare
@@ -64,15 +72,15 @@ func (c *CyberArkClient) PostDataReadingsWithOptions(ctx context.Context, payloa | |||
|
|||
presignedUploadURL, err := c.retrievePresignedUploadURL(ctx, hex.EncodeToString(checksum.Sum(nil)), opts) | |||
if err != nil { | |||
return err | |||
return fmt.Errorf("while retrieving snapshot upload URL: %s", err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This extra context around the error helped me know which part of the post operation was failing, during testing. It will also be helpful for us to diagnose problems if customers encounter this error path in the field.
I updated the unit tests accordingly.
if err != nil { | ||
return err | ||
} | ||
|
||
req.Header.Set("Content-Type", "application/json") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The AWS API is very picky about the request headers. With this content type header, the upload fails with the following error:
=== RUN TestPostDataReadingsWithOptionsWithRealAPI
identity.go:328: I0806 17:48:18.208472] made successful request to StartAuthentication source="Identity.doStartAuthentication" summary="NewPackage"
identity.go:444: I0806 17:48:18.599177] successfully completed AdvanceAuthentication request to CyberArk Identity; login complete username="[email protected]"
dataupload_test.go:172:
Error Trace: /home/richard/projects/jetstack/jetstack-secure/pkg/internal/cyberark/dataupload/dataupload_test.go:172
Error: Received unexpected error:
received response with status code 403: <?xml version="1.0" encoding="UTF-8"?>
<Error><Code>SignatureDoesNotMatch</Code><Message>The request signature we calculated does not match the signature you provided. Check your key and signing method.</Message><AWSAccessKeyId>ASIAR53UX6ITK7A63SVE</AWSAccessKeyId><StringToSign>PUT
application/json
1754499204
x-amz-security-token:IQoJb3JpZ2luX2VjEEEaCXVzLWVhc3QtMSJGMEQCID5ReOMYdbV6+xXUg1buIf83rg/BzGYJJMuBXB+CRk7ZAiBNC/fNLgruAsTg1NUkcdJ+Na63tmasBmp96D/GA4V8virTAwh6EAAaDDEzMjg1MTk1NDIxNCIM6RQ9ZMY
Test: TestPostDataReadingsWithOptionsWithRealAPI
--- FAIL: TestPostDataReadingsWithOptionsWithRealAPI (8.32s)
FAIL
FAIL github.com/jetstack/preflight/pkg/internal/cyberark/dataupload 8.336s
FAIL
Checksum string `json:"checksum_sha256"` | ||
ClusterID string `json:"cluster_id"` | ||
Checksum string `json:"checksum_sha256"` | ||
AgentVersion string `json:"agent_version"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed ClusterDescription and added AgentVersion, as per the API design.
@@ -120,7 +128,7 @@ func (c *CyberArkClient) retrievePresignedUploadURL(ctx context.Context, checksu | |||
|
|||
req.Header.Set("Content-Type", "application/json") | |||
if err := c.authenticateRequest(req); err != nil { | |||
return "", fmt.Errorf("failed to authenticate request") | |||
return "", fmt.Errorf("failed to authenticate request: %s", err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wanted to see the authentication error here to know what has failed.
The only error from the current implementation is no token cached
, which means that you've forgotten to call Login on the identity client.
ClusterName: "bb068932-c80d-460d-88df-34bc7f3f3297", | ||
}) | ||
require.NoError(t, err) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A successful run of this test looks like this:
$ go test ./pkg/internal/cyberark/dataupload/... -run Real -v -test.v 6
warning: both GOPATH and GOROOT are the same directory (/home/richard/go); see https://go.dev/wiki/InstallTroubleshooting
=== RUN TestPostDataReadingsWithOptionsReal
identity.go:328: I0806 17:14:42.059639] made successful request to StartAuthentication source="Identity.doStartAuthentication" summary="NewPackage"
identity.go:444: I0806 17:14:42.438060] successfully completed AdvanceAuthentication request to CyberArk Identity; login complete username="********@cyberark.cloud.****"
--- PASS: TestPostDataReadingsWithOptionsReal (2.67s)
PASS
ok github.com/jetstack/preflight/pkg/internal/cyberark/dataupload 2.681s
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") | ||
return | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The skip message looks like this:
$ go test ./pkg/internal/cyberark/dataupload/... -run Real -v
warning: both GOPATH and GOROOT are the same directory (/home/richard/go); see https://go.dev/wiki/InstallTroubleshooting
=== RUN TestPostDataReadingsWithOptionsWithRealAPI
dataupload_test.go:137: Skipping because one of the following environment variables is unset or empty: ARK_PLATFORM_DOMAIN, ARK_SUBDOMAIN, ARK_USERNAME, ARK_SECRET
--- SKIP: TestPostDataReadingsWithOptionsWithRealAPI (0.00s)
PASS
ok github.com/jetstack/preflight/pkg/internal/cyberark/dataupload 0.009s
pkg/version/version.go
Outdated
func SetUserAgent(req *http.Request) { | ||
req.Header.Set("User-Agent", UserAgent()) | ||
req.Header.Set("User-Agent", "Mozilla/5.0 "+UserAgent()) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The API team are aware of this and I will pester them to fix it.
… inventory API Signed-off-by: Richard Wall <[email protected]>
f2ae463
to
fa4fb98
Compare
Closes: https://venafi.atlassian.net/browse/VC-43753
dataupload
package for compatibility with the CyberArk Discovery and Context Inventory API, which is now available for testing.Testing
I added a (temporary) integration test to demonstrate the dataupload client working with the real API
Here's the sequence of API calls:

Here's a response from the snapshot-links endpoint:

Here's the PUT request to the AWS presigned URL: