Add backend authentication for targetRefs on vmusers by secret adapt …

[mohammadkhavari]

This pull request will add backend basic authentication support on vmuser to add them as header on vmauth configuration. if this approach is accepted I'll add other authentication formats (like bearer) if needed. https://github.com/VictoriaMetrics/operator/issues/669

[Haleygo]

Hello, we are trying to unify the auth config with HTTPAuth, can you reuse it on here? https://github.com/VictoriaMetrics/operator/blob/f2b8cf701a33f91cef19848c857fd6efb7db59dd/api/v1beta1/additional.go#L211-L223

[mohammadkhavari]

Sure.

[mohammadkhavari]

I'll fix this.

[mohammadkhavari]

I faced some issues about using HttpAuth in VMuser definition. It is clear that HttpAuth should place [inline] into TargetRef as the feature request is mentioning it will be transform to the targetRef headers. And all properties in VMuser's finally should be transformed to vmauth configuration, here goes the challenges I have in writing the logic of this transformation for each field of HttpAuth:

1. Headers: as it is present already in the targetRef struct it will override the outer Header field, the logic could remain the same . but the description will override by the HttpAuth Headers description that is mentioning wrong format of headers format:

HTTPAuth Headers: semicolon separated header with value e.g. headerName:headerValue TargetRef Headers: ["header_key: value1,value2"]

2. BasicAuth, bearerToken: It can be implemented using headers configurations the only drawback are passwordFiles field, they usage is specified for CRDs that are leading to deploy a pod and there's filesystem so we can refer a file. But here there's another operation.

3. About TLSConfig, Oauth: as I have investigated these cannot be satisfy only through headers or current vmauth configuration we have here for each user.

And the logic behind this transformation in something like vmalert is much simpler and straightforward: vmalert crd implements HTTPAuth in remoteWrite section and as we can see theres options(https://docs.victoriametrics.com/vmalert.html ) like -remoteWrite.oauth2.clientID string in vmalert component configuration and operator is reshaping the input configs to component flags.

my recommendation is to first implement HttpAuth features in vmauth component (as I have created issue for basicAuth) and support them in configuration formats then update operator.

What do you think? @Haleygo @f41gh7

And also what is your opinion on passwordFiles? inline basicAuth on CRDS in something like vmalert.notifier.basicAuth.password will finally transform to datasource.basicAuth.passwordFile=/etc/vmalert/remote_secrets/DATASOURCE_BASICAUTHPASSWORD on vmalert component args to hide them in prior info, I think vmauth needs some configuration like this.

[Haleygo]

I faced some issues about using HttpAuth in VMuser definition.

@mohammadkhavari Thank you for all the work! Sorry I didn't look too deep when I propose that. Now I think it's ok to just use BasicAuth here like you already did, I might do some follow up when I work on this later.

[mohammadkhavari]

Thank you, Please inform me if any refactoring, code completion, or other modifications, such as adding tests or fixing code, are required. @Haleygo

[mohammadkhavari]

@Haleygo Are there any updates on the feature?

[mohammadkhavari]

@Haleygo @Amper Hi, I've updated the implementation due to the suggested interface. A test case has been added, and api.md has been updated as well. I would appreciate it if you could review my changes.

[f41gh7]

@Haleygo @Amper Hi, I've updated the implementation due to the suggested interface. A test case has been added, and api.md has been updated as well. I would appreciate it if you could review my changes.

Thanks! I'll take a look soon and most probably, it'll a part of upcoming release.

[f41gh7]

Thanks for contribution!

[mohammadkhavari]

@f41gh7 According to the failed pipeline, Ive noticed that I missed the refactor structure commit, I have fixed implementations and adopt new interfaces by a new separated commit. so, make lint make test are also passing locally. https://github.com/mohammadkhavari/operator/commit/a63743a17ab3202611818f953090e4832d13fe20

should I make another pull req?

[f41gh7]

@f41gh7 According to the failed pipeline, Ive noticed that I missed the refactor structure commit, I have fixed implementations and adopt new interfaces by a new separated commit. so, make lint make test are also passing locally. mohammadkhavari@a63743a

should I make another pull req?

No worries, I ll make follow-up commit with fix for it.