Won't this cause the test to fail when run on any other project?

Also, why is there not a $ at the start of the value?

When does the test run on other projects?

"Also, why is there not a $ at the start of the value?" -> fixed, ty.

When does the test run on other projects?

When it's run directly from a developer's workstation with go test

The secrets are accessible as long as the project hosting the secrets grants the roles/secretmanager.secretAccessor to the test VM Service Account.

If the test is not running from within a VM, then the deverlopers @google.com email needs to be granted the iam role.

And if the developer does not have an @google.com account? Ops Agent is open source software.

We should probably add a test for this case specifically.

Nit. Wouldn't we want secretPassword to start with ${googlesecretmanager? (i.e. HasPrefix)

Currently, we are trying to prevent users from using the secret provider in OracleDB-related configuration entries.

This is because the password will be URI-escaped before being passed to the OTel collector, which results in the provider identifier being altered and consequently unable to map back to a secret entry in Google Secret Manager.

We will remove this check later when we implement the fix in the sqlquery receiver.

Here's the upstream issue: open-telemetry/opentelemetry-collector-contrib#40117

This isn't an upstream issue; an upstream user can and should just put an escaped string into the secret.

Copied from my comment in the upstream issue:

I agree that the "ideal" solution is to have a sharable component that can be called to URL encode or escape sections of the configuration.

From a "where this belongs" perspective between the two existing components though, I think it's closer to the sql receiver than the secret manager. This escaping issue is not specific to the case of wanting to use the secret manager. It also applies to using env variable (in the example Braydon provided), or any other provider in the future.

The sql receiver also knows exactly what type of escaping it needs to do (in this case URL encoding).

Based on my previous comment, I don't think deciding whether to implement escaping in a new provider or sql receiver should be blocking for the Secret Manager integration.

Also copied from the upstream issue:

This isn't a question of the "ideal" solution - the proposed solution here can not work. Once the string is already concatenated into a single URL, it's no longer possible to know which part of that string needs encoding applied to it.

Also, the exact same problem applies to environment variable substitution, not just googlesecretmanager, so if you want to report a configuration error until this is supported upstream, you should be checking for ${ and not just ${googlesecretmanager

That is a good point. This would also not work with env variable substitution. Maybe we do a regex match for something like${.*}

Let's also change the error message to say that this integration does not support confmap providers.

Done

Why was the \EOF added in every enable script here?

Adding \ so that the provider identifier is not interpreted as a shell variable.

This enable file content is eventually stored as a shell script in the test VM:

Adding \ so that the provider identifier is not interpreted as a shell variable.

This does not explain why EOF has a \, I'm still confused about this.

The change in this file (adding \ in front of EOF) has been reverted.

I am not a fan of the testing strategy in this PR because:

1. It overrides the plaintext password testcases reducing our coverage
2. It buries googlesecretmanager tests in random database app tests. It should really have its own dedicated test cases in the main test framework.

I think this should be redone in a way that addresses both of those points.

I agree with this, and I think it should be replaced with an integration test that also creates the secrets in the current project, so the test is not hardcoded to a single project.

Yeah that would make more sense given how other tests in this repo work, given that none of the others hardcode a project.

Done