[Merged by Bors] - Add support for LDAP authentication

[sbernauer]

Description

Superseding #145, completely reworked to use new configuration mechanism introduced in #173
For #5
Integration-Tests: https://github.com/stackabletech/integration-tests/pull/175
Use ./create_test_cluster.py --kind --operator superset=0.4.0-pr180 before running the tests

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

[sbernauer]

Integration tests are green => ready for review :)

[maltesander]

Just a small thing. Ran the tests and works. Maybe @siegfriedweber can have another look? Im not that into ldap and superset ;)

@sbernauer i cannot judge how good i is, but since we will require ldap etc. in other operators to i think its fine to merge in a working approach and refine / improve that later as well.

[siegfriedweber]

There are some minor and one major issue left.

The major issue is that some views are not accessible by the administrator, e.g. the user list. The reason is that the Admin role does not have the UserLDAPModelView permission. The command superset init which is called in the job created by the superset_db_controller initializes the roles according to the configured authentication type. If the authentication type is AUTH_LDAP then the permission UserLDAPModelView is added to the Admin role. Unfortunately the authentication type is not set in the configuration created by the superset_db_controller and it is not trivial to add it.

[sbernauer]

The major issue is that some views are not accessible by the administrator, e.g. the user list.

The problem should now be sorted out via 458b7f2

The problem with the first login failing is purely a bug in flask-appbuilder and should be fixed by dpgaspar/Flask-AppBuilder#1846 which is entirely a docker image thing and not part of this PR.

Could you please take another look - would be great before your holiday to get this in 😇

[sbernauer]

@adwk67, @siegfriedweber was fine with the current implementation and requested fixing the "init container must finish before Superset starts" before merging.
Could you please took a look at the last 2 commits c60740a and 6df5300 and review them?
I tested the LDAP integration and for me everything worked fine and logs were ok

[adwk67]

lgtm. Opened a separate issue #195 to avoid db-clashes in the tests

[sbernauer]

Thanks all for your feedback!
Everything should work to the best of my knowledge. Together with stackabletech/docker-images#116 this seems to be a nice solution :)
The Authorization part is outstanding and will be handled via a separate PR.

[sbernauer]

bors r+

[bors]

👎 Rejected by code reviews

Dismissing this due to Sigi being on vacation for 2 weeks and subsequent reviews by other people having confimed his concerns as addressed.

[sbernauer]

bors r+

[bors]

Pull request successfully merged into main.

Build succeeded:

• All tests passed