Feature/eso patch

[EldarShalev]

I've implemented the ESO (External Secrets Operator) CRDs collection following the same minimal pattern as the ConfigMaps feature #769 .

update: seemts to work locally and include the new fields

Here's what was done:
Files Modified:
deploy/charts/disco-agent/templates/configmap.yaml
Added two new k8s-dynamic data gatherers:
ark/externalsecrets (group: external-secrets.io, version: v1, resource: externalsecrets)
ark/secretstores (group: external-secrets.io, version: v1, resource: secretstores)
No label selectors used (Option A: collect all in watched namespaces)

deploy/charts/disco-agent/tests/snapshot/configmap_test.yaml.snap
Updated all 4 test snapshot sections to include both ESO gatherers

examples/machinehub.yaml
Added both ESO gatherers with comments explaining their purpose

examples/machinehub/input.json
Added empty items entries for both ark/externalsecrets and ark/secretstores

internal/cyberark/dataupload/dataupload.go
Added ExternalSecrets []runtime.Object and SecretStores []runtime.Object fields to the Snapshot struct

pkg/client/client_cyberark.go
Added extractor functions for ark/externalsecrets and ark/secretstores

pkg/client/client_cyberark_test.go
Added both gatherers to defaultDynamicDatagathererNames list

pkg/client/client_cyberark_convertdatareadings_test.go
Added TestConvertDataReadings_ExternalSecrets test
Added TestConvertDataReadings_SecretStores test
Both tests verify: 2 resources are extracted, deleted resources are excluded
Files Created:
hack/ark/external-secret.yaml
Sample ExternalSecret CR for e2e testing (uses fake backend)
hack/ark/secret-store.yaml
Sample SecretStore CR for e2e testing (uses fake provider)
hack/ark/test-e2e.sh
Updated to apply both ESO sample manifests (with fallback if CRDs not installed)

[achuchev]

The PR looks promising. I think the main area for improvement is implementing logic to exclude sensitive data. Unfortunately, the spec.provider.fake.data may include passwords in the manifest. Although the Fake provider is intended for testing purposes only, it still contains customer-sensitive data.

The potentially sensitive branch could be removed by applying the same approach used for managedFields.

[achuchev]

I am unable to find where the ESO CRDs have been deployed. If the CRDs are not in place, the CR will also not be deployed, which does not bring much value for the e2e.

[achuchev]

Suggested change

	"ark/externalsecrets": func(r *api.DataReading, s *dataupload.Snapshot) error {
	"ark/esoexternalsecrets": func(r *api.DataReading, s *dataupload.Snapshot) error {

It may be helpful to add a prefix to the names to clarify which project they originate from.

[EldarShalev]

sure, but I guess you dont mean only here right?
just to understand the scope of the prefix

[achuchev]

The name should be updated everywhere it appears (incl. tests). This applies to all new data gatherers.

[EldarShalev]

Done

[inteon]

@EldarShalev could you run make fix-golangci-lint?

[EldarShalev]

@EldarShalev could you run make fix-golangci-lint?

will try to fix it, getting

make/connection_crd/main.go:6:6: could not import github.com/jetstack/venafi-connection-lib/config/crd/bases (make/connection_crd/main.go:6:2: reading github.com/jetstack/venafi-connection-lib/go.mod at revision v0.5.2: git ls-remote -q origin in /Users/eldar.shalev/go/pkg/mod/cache/vcs/e1445deb79b4ca14ee954e0f79f4034748b368f02a81510920a656b6ce2c6380: exit status 128:
        ERROR: Repository not found.
        fatal: Could not read from remote repository.
        
        Please make sure you have the correct access rights
        and the repository exists.) (typecheck)
        crd "github.com/jetstack/venafi-connection-lib/config/crd/bases"

[EldarShalev]

should it be v1 or v1beta1?
@achuchev @SgtCoDFish

[SgtCoDFish]

I think for a test, it's fine to just test v1!

[SgtCoDFish]

suggestion: This seems like a mistake. Changing the resource version for one CRD shouldn't require changes to any other CRDs.

This - and many of the other changes in this file - should be reverted. We shouldn't have to change the "googlecasissuers" resource, nor should we change the AWS PCS issuer or other resources unrelated to ESO.

This is the reason that the "tests / test (pull_request)" job is failing on this PR.

[EldarShalev]

I will revert the changes for configmap, thanks

[SgtCoDFish]

I believe the ark-test-e2e job is failing because you've raised the PR from a fork (which - to be clear - was the right thing to do!)

I think the repo is set up in such a way that the e2e isn't runnable on forks. I'm digging into it a bit more