Support discoveryProperties in Configuration

[johnsonshih]

What this PR does / why we need it:

Discovery handlers need credential data for authenticated discovery. This PR allows users to specify credential data through discoveryProperties, Agent read the properties and pass them to discovery handlers.

This PR implements the proposal: project-akri/akri-docs#61
Special notes for your reviewer:

If applicable:

• this PR has an associated PR with documentation in akri-docs
• this PR contains unit tests
• added code adheres to standard Rust formatting (cargo fmt)
• code builds properly (cargo build)
• code is free of common mistakes (cargo clippy)
• all Akri tests succeed (cargo test)
• inline documentation builds (cargo doc)
• all commits pass the DCO bot check by being signed off -- see the failing DCO check for instructions on how to retroactively sign commits

[diconico07]

That's a nice feature here, the missing part is the validating webhook thing.

I also noticed you do the parameters gathering per discoveryEndpoints, it might be more efficient to do it only once at discoveryOperator's creation ?

[johnsonshih]

I also noticed you do the parameters gathering per discoveryEndpoints, it might be more efficient to do it only once at discoveryOperator's creation ?

I thought about that but later changed to ready the secret information at the time establishing the endpoint connection. Reason:

1. provide as much as up-to-date secret data, in case the secret data changed, this gives agent a chance to refresh the secret data.
2. DiscoveryOperator is a long run entity, don't want to keep the secret data in memory for a long time.

[bfjelds]

curious, why this would be success case?

[johnsonshih]

we don't restrict empty name, as long as it's not None, we accept it (although it probably won't be used when DH query the secret data).

[bfjelds]

seems strange to me to allow an empty name

[bfjelds]

i think an empty name should be an error, but won't block the PR based on that.

[johnsonshih]

if we treat empty name as error, should we treat name with all spaces as error, too? IMO, from syntax perspective, an empty string is a valid string and is valid as a key in key-value pair. We can allow it.
BTW, a null string for name is for sure not allowed, it's specified in the schema

[bfjelds]

i dont think an empty string and a string with spaces are the same.

[diconico07]

I'd be in favor of specifying name in the same way it is defined in core EnvVar, that is as a C_IDENTIFIER (i.e a string conforming to this regex: [A-Za-z_][A-Za-z0-9_]* thus disallowing empty names and ensuring these discovery properties can be passed as environment variable if need arise.

I don't know if there is a way to translate that in the CRD or if we can only rely on the webhook to reject configurations that are in this case.

[johnsonshih]

yes, we can put pattern in the crd and not relies on webhook to validate the content. I'll investigate if we really want to make that hard restriction to the key name.

[johnsonshih]

filed Issue for the investigation effort.
#634

[diconico07]

I won't block the PR for not triggering errors on empty names, but please create an issue for doing that later if you don't do it in this PR.