[VC-43403] CyberArk(agent): add support for MachineHub output mode #696
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
inteon
reviewed
Aug 22, 2025
wallrj-cyberark
force-pushed
the
VC-43403-fix-output-path-mode
branch
4 times, most recently
from
bcb07ec to
4e5623d
Compare
wallrj-cyberark
force-pushed
the
VC-43403-client
branch
from
949f238 to
3259d28
Compare
maelvls
reviewed
Aug 22, 2025
wallrj-cyberark
force-pushed
the
VC-43403-client
branch
6 times, most recently
from
2690901 to
78700c9
Compare
wallrj-cyberark
changed the base branch from
master
to
VC-43403-servicediscovery-inventory-api
This was referenced Aug 27, 2025
wallrj-cyberark
force-pushed
the
VC-43403-servicediscovery-inventory-api
branch
from
ff781e3 to
b82d7ce
Compare
wallrj-cyberark
force-pushed
the
VC-43403-client
branch
from
78700c9 to
b7adba8
Compare
This was referenced Aug 28, 2025
wallrj-cyberark
force-pushed
the
VC-43403-servicediscovery-inventory-api
branch
3 times, most recently
from
78859a8 to
1a40e3f
Compare
Base automatically changed from
VC-43403-servicediscovery-inventory-api
to
master
August 28, 2025 12:11
wallrj-cyberark
force-pushed
the
VC-43403-client
branch
from
b7adba8 to
c51220b
Compare
wallrj-cyberark
force-pushed
the
VC-43403-client
branch
from
c51220b to
d35b7cb
Compare
wallrj-cyberark
force-pushed
the
VC-43403-client
branch
2 times, most recently
from
a331e26 to
0bfb107
Compare
wallrj-cyberark
changed the title
wallrj-cyberark
force-pushed
the
VC-43403-client
branch
4 times, most recently
from
ae1b4fb to
d82113d
Compare
wallrj-cyberark
commented
Aug 28, 2025
| type Options struct { | ||
| ClusterName string | ||
| } | ||
|
|
There was a problem hiding this comment.
Forgot to remove this in #703
| ClusterID: "bb068932-c80d-460d-88df-34bc7f3f3297", | ||
| }) | ||
| require.NoError(t, err) | ||
| } |
There was a problem hiding this comment.
The code used to setup the dataupload wrapper in this test has now been moved to pkg/internal/cyberark, so I've moved this test there and updated it to simply use the new NewDatauploadClient function.
There was a problem hiding this comment.
I'll add more unit tests here in #684 when I implement the conversion between DataReadings and Snapshot.
In a future PR I might also remove the "RealAPI" test which mostly duplicates the same test in pkg/internal/cyberark/client_test.go.
| @@ -28,7 +28,7 @@ const ( | |||
| // mockSuccessfulStartAuthenticationToken is the token returned by the | |||
| // mock server in response to a successful AdvanceAuthentication request | |||
| // Must match what's in testdata/advance_authentication_success.json | |||
| mockSuccessfulStartAuthenticationToken = "long-token" | |||
| mockSuccessfulStartAuthenticationToken = "success-token" | |||
There was a problem hiding this comment.
All the fake values in the various mocks have to be consistent for the testutil.FakeCyberArk combined mock to work.
In a future PR I might try and fix that.
wallrj-cyberark
force-pushed
the
VC-43403-client
branch
from
d82113d to
8842333
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR introduces CyberArk MachineHub integration for the Venafi Kubernetes Agent by adding a new output mode that publishes data readings to CyberArk's discoverycontext API. The implementation includes service discovery, authentication, and data upload capabilities for CyberArk's platform.
- Added MachineHub output mode with
--machine-hubflag for CyberArk integration - Implemented CyberArk client with service discovery, identity authentication, and data upload
- Created environment-based configuration loading for CyberArk credentials
- Updated test infrastructure to support mock CyberArk APIs
Reviewed Changes
Copilot reviewed 16 out of 17 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/testutil/envtest.go | Added FakeCyberArk helper for testing with mock CyberArk APIs |
| pkg/internal/cyberark/servicediscovery/mock.go | Changed MockDiscoveryServer parameter from *testing.T to testing.TB |
| pkg/internal/cyberark/servicediscovery/discovery_test.go | Updated test to use exported IdentityServiceName constant |
| pkg/internal/cyberark/servicediscovery/discovery.go | Exported service name constants and cleaned up formatting |
| pkg/internal/cyberark/identity/testdata/advance_authentication_success.json | Updated mock token value to match test expectations |
| pkg/internal/cyberark/identity/mock.go | Updated mock token constant to match test data |
| pkg/internal/cyberark/dataupload/dataupload_test.go | Removed real API test and cleaned up imports |
| pkg/internal/cyberark/dataupload/dataupload.go | Removed unused Options struct |
| pkg/internal/cyberark/client_test.go | Added comprehensive tests for CyberArk client functionality |
| pkg/internal/cyberark/client.go | Implemented CyberArk client with environment-based configuration |
| pkg/client/client_cyberark_test.go | Added tests for CyberArk client integration |
| pkg/client/client_cyberark.go | Implemented CyberArk client wrapper for agent integration |
| pkg/agent/config_test.go | Added tests for MachineHub mode configuration and validation |
| pkg/agent/config.go | Integrated MachineHub mode into agent configuration system |
| examples/machinehub.yaml | Added example configuration file for MachineHub mode |
| .envrc.template | Added CyberArk environment variable template |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
- Introduced a new `MachineHub` output mode in the agent configuration. - Added `--machine-hub` flag to enable the `MachineHub` mode. - Implemented `CyberArkClient` for publishing data readings to CyberArk's API. - Created `LoadClientConfigFromEnvironment` to load CyberArk client configuration from environment variables. - Updated tests to cover `MachineHub` mode and CyberArk client functionality. - Modified mock data and discovery logic to support CyberArk integration. Signed-off-by: Richard Wall <[email protected]>
wallrj-cyberark
force-pushed
the
VC-43403-client
branch
from
8842333 to
5a2ae3c
Compare
maelvls
reviewed
Aug 29, 2025
Comment on lines
+270
to
+272
| // The environment variable `ARK_DISCOVERY_API` is set to the URL of the mock | ||
| // Service Discovery API, for the supplied `testing.TB` so that the client under | ||
| // test will use the mock Service Discovery API. |
There was a problem hiding this comment.
Not a big deal, but ARK_DISCOVERY_API seems to default to https://platform-discovery.integration-cyberark.cloud/api/v2, but doesn't default to anything in the mock... I found that because I had forgotten to set ARK_DISCOVERY_API, and was super confused by the following output:
Without ARK_DISCOVERY_API:
I0829 10:39:18.377748 32025 round_trippers.go:632] "Response" logger="Run.gatherAndOutputData.postData" verb="GET" url="https://platform-discovery.cyberark.cloud/api/v2/services/subdomain/tlskp-test" status="404 Not Found" milliseconds=1063
I0829 10:39:18.377812 32025 run.go:334] "Warning: PushingErr: retrying" logger="Run.gatherAndOutputData" in="22.148195504s" reason="post to server failed: while initializing data upload client: got an HTTP 404 response from service discovery; maybe the subdomain \"tlskp-test\" is incorrect or does not exist?"
With ARK_DISCOVERY_API:
I0829 10:40:45.658647 33943 round_trippers.go:632] "Response" logger="Run.gatherAndOutputData.postData" verb="GET" url="https://platform-discovery.integration-cyberark.cloud/api/v2/services/subdomain/tlskp-test" status="200 OK" milliseconds=212
I0829 10:40:46.155025 33943 round_trippers.go:632] "Response" logger="Run.gatherAndOutputData.postData" verb="POST" url="https://anb5751.id.integration-cyberark.cloud/Security/StartAuthentication" status="200 OK" milliseconds=495
IMO the mock should either use a default ARK_DISCOVERY_API value if missing, or it should fail and tell the user to set ARK_DISCOVERY_API. I might have misunderstood though.
maelvls
approved these changes
Aug 29, 2025
|
Yes, I was able to run the agent with a real configuration: Config was: data-gatherers:
- kind: "k8s-discovery"
name: "k8s-discovery"
- kind: "k8s-dynamic"
name: "k8s/pods"
config:
resource-type:
resource: pods
version: v1
- kind: "k8s-dynamic"
name: "k8s/services"
config:
resource-type:
resource: services
version: v1
- kind: "k8s-dynamic"
name: "k8s/deployments"
config:
resource-type:
version: v1
resource: deployments
group: apps
- kind: "k8s-dynamic"
name: "k8s/replicasets"
config:
resource-type:
version: v1
resource: replicasets
group: apps
- kind: "k8s-dynamic"
name: "k8s/statefulsets"
config:
resource-type:
version: v1
resource: statefulsets
group: apps
- kind: "k8s-dynamic"
name: "k8s/daemonsets"
config:
resource-type:
version: v1
resource: daemonsets
group: apps
- kind: "k8s-dynamic"
name: "k8s/jobs"
config:
resource-type:
version: v1
resource: jobs
group: batch
- kind: "k8s-dynamic"
name: "k8s/cronjobs"
config:
resource-type:
version: v1
resource: cronjobs
group: batch
- kind: "k8s-dynamic"
name: "k8s/ingresses"
config:
resource-type:
group: networking.k8s.io
version: v1
resource: ingresses
- kind: "k8s-dynamic"
name: "k8s/secrets"
config:
resource-type:
version: v1
resource: secrets
- kind: "k8s-dynamic"
name: "k8s/certificates"
config:
resource-type:
group: cert-manager.io
version: v1
resource: certificates
- kind: "k8s-dynamic"
name: "k8s/certificaterequests"
config:
resource-type:
group: cert-manager.io
version: v1
resource: certificaterequests
- kind: "k8s-dynamic"
name: "k8s/issuers"
config:
resource-type:
group: cert-manager.io
version: v1
resource: issuers
- kind: "k8s-dynamic"
name: "k8s/clusterissuers"
config:
resource-type:
group: cert-manager.io
version: v1
resource: clusterissuers
- kind: "k8s-dynamic"
name: "k8s/googlecasissuers"
config:
resource-type:
group: cas-issuer.jetstack.io
version: v1beta1
resource: googlecasissuers
- kind: "k8s-dynamic"
name: "k8s/googlecasclusterissuers"
config:
resource-type:
group: cas-issuer.jetstack.io
version: v1beta1
resource: googlecasclusterissuers
- kind: "k8s-dynamic"
name: "k8s/awspcaissuer"
config:
resource-type:
group: awspca.cert-manager.io
version: v1beta1
resource: awspcaissuers
- kind: "k8s-dynamic"
name: "k8s/awspcaclusterissuers"
config:
resource-type:
group: awspca.cert-manager.io
version: v1beta1
resource: awspcaclusterissuers
- kind: "k8s-dynamic"
name: "k8s/mutatingwebhookconfigurations"
config:
resource-type:
group: admissionregistration.k8s.io
version: v1
resource: mutatingwebhookconfigurations
- kind: "k8s-dynamic"
name: "k8s/validatingwebhookconfigurations"
config:
resource-type:
group: admissionregistration.k8s.io
version: v1
resource: validatingwebhookconfigurations
- kind: "k8s-dynamic"
name: "k8s/gateways"
config:
resource-type:
group: networking.istio.io
version: v1alpha3
resource: gateways
- kind: "k8s-dynamic"
name: "k8s/virtualservices"
config:
resource-type:
group: networking.istio.io
version: v1alpha3
resource: virtualservices
- kind: "k8s-dynamic"
name: "k8s/routes"
config:
resource-type:
version: v1
group: route.openshift.io
resource: routes
- kind: "k8s-dynamic"
name: "k8s/venaficlusterissuers"
config:
resource-type:
group: jetstack.io
version: v1alpha1
resource: venaficlusterissuers
- kind: "k8s-dynamic"
name: "k8s/venafiissuers"
config:
resource-type:
group: jetstack.io
version: v1alpha1
resource: venafiissuers
- kind: "k8s-dynamic"
name: "k8s/fireflyissuers"
config:
resource-type:
group: firefly.venafi.com
version: v1
resource: issuersHTTP request didn't contain anything, though: PUT /8f08a102-58ca-49cd-960e-debc5e0d3cd4/success-cluster-id/20250829_092808_929.snapshot?AWSAccessKeyId=ASIAR53UX6ITONJKSTTA&Signature=asJm5rh1avp%2FjXLTiSy56uRfej8%3D&x-amz-tagging=agent_version%3Ddevelopment%26uploader_id%3Dsuccess-cluster-id%26vendor%3Dk8s%26checksum_sha256%3De553fd7f2b2d931b8dafabd0f58d1ef93e0479c58df7a96ed54537e09b3106e7%26tenant_id%3D8f08a102-58ca-49cd-960e-debc5e0d3cd4%26username%3Dmael.valais%40cyberark.cloud.420375&x-amz-server-side-encryption=AES256&x-amz-checksum-sha256=e553fd7f2b2d931b8dafabd0f58d1ef93e0479c58df7a96ed54537e09b3106e7&x-amz-security-token=IQoJb3JpZ2luX2VjEGIaCXVzLWVhc3QtMSJHMEUCIQCPOAOkByV5GZbxcTeCaslTYSz%2FPpgH%2FnRAAHzpBwYGfQIge8nGnjkxF7TAgVffLDWYoF0mVz8zCMinYfe25Z4MtrAq3AMIu%2F%2F%2F%2F%2F%2F%2F%2F%2F%2F%2FARAAGgwxMzI4NTE5NTQyMTQiDJ0ipvG0hTLgpkdYzCqwA1kkAurXUEn00DMOQS503TpIBeF29cAKs%2FuYpz714a738X6LkWk6Vd59xXKVH3Z7Way1aixn4M3bfebXk0GMirYITvjrhdMyB%2B7Xq2HW4WCVXa%2Fg8DD4krc8UAnIpX4tHiuZEuNo%2FG5ouu4Gb3uPilFPxAAeOR3qqQAsyh3S6Eai8WERk%2BZY6lf%2BGTkNn7p3q5YzS0WD%2BtovJ3EHD4gy4puo5Ohc28eoUOKTRpK8OHfkzOYwmuVwxNihN3Sc8SGXdEkyO5j6QJ8A7RfdagTgJONBF39zerpL1jLsbI8q5Y8WfySn%2FRoqqXxLsZK5r5zBY1Ha%2BjcgZT%2BTrI1SM3SvjA7luDbaWw39a3ox08jPNZsZCTQw1ZNbfmdf9e43QAWiY%2BD45abivhp1SngSBBdCypvvXpruUDaFUyOYKi2c3JCaAyPo9r92FG%2BW7VVAqyghxWSKxg66ikk1IuPvaSwrgDZV6pkVyEfEDxWBm%2FgHIxcvDDPHUNGBbgZShqPkFxBFrvdRrkwNM3Uw0WI3oB3SJCeKqwUYHhUjkB0KAMOznpLlQEgxCa7mwtOnDSskCYJqmzDi48XFBjqeAdrFypD5bRcdryuGJEkAKWWF5RYKSDAdW%2BWImYYsIu0fpcVPbE7dB%2FGZqam7G8udKAPGKOswOWS0kwCW3DznJbhJvLcvg7GEBBOfdIj30iNWLFWyBBfcXiA1%2B8IfDOSAFtinXhjMRHdg%2FELuua4mQUhJmqAd9NBUSmSkTnEGmZ7Cxn1Uq9q%2FslM7pf2etGIkeh4F%2FgB6qVDqliG7SYir&Expires=1756459989 HTTP/1.1
Host: kubernetes-snapshots-marshmallow-a1b2c3d4-integration.s3.amazonaws.com
User-Agent: Mozilla/5.0 venafi-kubernetes-agent/development
Content-Length: 298
X-Amz-Checksum-Sha256: 5VP9fystkxuNr6vQ9Y0e+T4EecWN96lu1UU34JsxBuc=
Accept-Encoding: gzip
{
"agent_version": "development",
"cluster_id": "success-cluster-id",
"k8s_version": "",
"secrets": null,
"serviceaccounts": null,
"roles": null,
"clusterroles": null,
"rolebindings": null,
"clusterrolebindings": null,
"jobs": null,
"cronjobs": null,
"deployments": null,
"statefulsets": null,
"daemonsets": null,
"pods": null,
} |
|
Thanks @maelvls Yes, no data is expected. In the next PR I've got a function to convert between the data readings and the snapshot format. In a future PR I will add the service discovery endpoint URL to the error message |
|
By the way, is it expected that each "push" requires re-authenticating to the API? I don't think it is a big deal, though. 
Note for later: seems like we will have to update the docs to remind customers that they will have to allow two domains:
|
Yes, that's expected for now. There's no mechanism in the identity wrapper to use refresh-tokens...and in any case, the plan is for the Disco agent to upload every 12h, so I don't think it matters too much. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
CyberArk(agent): add support for MachineHub output mode:
MachineHuboutput mode in the agent configuration.--machine-hubflag to enable theMachineHubmode.CyberArkClientfor publishing data readings to CyberArk's API.LoadClientConfigFromEnvironmentto load CyberArk client configuration from environment variables.MachineHubmode and CyberArk client functionality.Followup PRs
Testing
This gets us to the point of being able to run the agent in machine hub mode from the command line, as follows:
go run . agent --one-shot --machine-hub -v 6 --agent-config-file ./examples/machinehub.yaml