Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: initial version of the ldap interface #16

Merged
merged 2 commits into from
Oct 26, 2023
Merged

Conversation

wood-push-melon
Copy link
Contributor

This pull request tries to present the preliminary version of the juju charm library for the ldap juju interface. This version implements fundamental provider, consumer, and juju custom events.

Note that this juju charm library is subject to change once the design of the glauth-k8s charm is finalized.

@wood-push-melon wood-push-melon requested a review from a team October 26, 2023 02:27
Copy link
Contributor

@nsklikas nsklikas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally we use jsonschema or Pydantic to validate and parse the databag's content. The databag is pretty simple right now, so I have no problem with not including that kind of validation rn.

Overall LGTM

lib/charms/glauth_k8s/v0/ldap.py Outdated Show resolved Hide resolved
lib/charms/glauth_k8s/v0/ldap.py Outdated Show resolved Hide resolved
Copy link
Contributor

@nsklikas nsklikas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As we discussed, we need to implement a update_app_data (or something along those lines) method in LdapProvider, that will be used by the charm to update the ldap_uri and base_dnin all relations.

We can merge it without that, but please create an issue to keep track of it.

@wood-push-melon
Copy link
Contributor Author

As we discussed, we need to implement a update_app_data (or something along those lines) method in LdapProvider, that will be used by the charm to update the ldap_uri and base_dnin all relations.

We can merge it without that, but please create an issue to keep track of it.

Yep, will convert my local TODO list to some JIRA tickets for the memo.

@wood-push-melon wood-push-melon merged commit 5f18a7e into main Oct 26, 2023
3 checks passed
@wood-push-melon wood-push-melon deleted the IAM-287 branch October 26, 2023 20:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants