[Merged by Bors] - fixup ability to detect MAP BOX API key and add it to the environment…

[rsiwicki]

… if present.

Description

Addition of ability to add the lookup for the MAP_BOX_API key that Superset can use to obtain geo information and map backgrounds for lat / long style queries.

Review Checklist

• Code contains useful comments
• (Integration-)Test cases added (or not applicable)
• Documentation added (or not applicable)
• Changelog updated (or not applicable)
• Cargo.toml only contains references to git tags (not specific commits or branches)
• Helm chart can be installed and deployed operator works (or not applicable)

Once the review is done, comment bors r+ (or bors merge) to merge. Further information

[stackable-bot]

All committers have signed the CLA.

[razvan]

Hi Rob, thanks for your PR. Unless I'm missing something, this can be reduced to a single call like this:

cb.add_env_var_from_secret("MAPBOX_API_KEY", &value, "connections.mapBoxApiKey");

similar to the way it's done in lines 401 and 402.

[sbernauer]

I think @razvan's suggestion is the way to go. Here are also some points that might interest you describing why the current approach of the PR is not optimal. Hope they are helpful :)

1. The credentials (api key) will be written in clear text in the StatefulSet Manifest which can be pretty dangerous
2. The operator now reads the credentials, which we try to avoid
3. We would have to watch the secret for changes and reconcile. When we just reference the Secret as suggested by @razvan restart-controller handles this for us automatically

[rsiwicki]

@sbernauer @razvan - have simplified this to use optional crd property for mapbox api key.

[razvan]

lgtm

[razvan]

bors merge

[bors]

Pull request successfully merged into main.

Build succeeded:

• All tests passed

[sbernauer]

Great, thanks!
I just noticed that the Env Variable gets set but the superset_config.py doesn't use it. Shouldn't we need to add something like MAPBOX_API_KEY = os.environ.get("MAPBOX_API_KEY") to the superset_config.py?

[rsiwicki]

@sbernauer - yes I think that would make sense.