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

Add settings for doing secure auth LDAPS/AD #391

Merged
merged 9 commits into from
Mar 8, 2024

Conversation

wrender
Copy link
Contributor

@wrender wrender commented Feb 15, 2024

Q A
Bug fix? yes
New feature? no
API breaks? no
Deprecations? no
Related tickets fixes #X, partially #Y, mentioned in #Z
License Apache 2.0

What's in this PR?

Adding settings required to do secure LDAP with nifi

Why?

Missing settings for doing secure LDAP

Checklist

  • Implementation tested
  • Error handling code meets the guideline
  • Logging code meets the guideline
  • User guide and development docs updated (if needed)
  • Append changelog with changes

To Do

  • If the PR is not complete but you want to discuss the approach, list what remains to be done here

@wrender wrender changed the title add ldaps settings Add settings missing for doing secure LDAP/AD Feb 15, 2024
@wrender wrender changed the title Add settings missing for doing secure LDAP/AD Add settings for doing secure LDAP/AD Feb 15, 2024
@mh013370
Copy link
Member

Would you mind updating the LDAP documentation here for the new configurable fields?

https://github.com/konpyutaika/nifikop/blob/master/site/docs/5_references/1_nifi_cluster/1_nifi_cluster.md?plain=1#L189-L196

@wrender
Copy link
Contributor Author

wrender commented Feb 16, 2024

Would you mind updating the LDAP documentation here for the new configurable fields?

https://github.com/konpyutaika/nifikop/blob/master/site/docs/5_references/1_nifi_cluster/1_nifi_cluster.md?plain=1#L189-L196

Ok. Done

@wrender wrender changed the title Add settings for doing secure LDAP/AD Add settings for doing secure auth LDAPS/AD Feb 18, 2024
api/v1/nificluster_types.go Outdated Show resolved Hide resolved
- indent fix

Co-authored-by: Juldrixx <31806759+juldrixx@users.noreply.github.com>
@wrender
Copy link
Contributor Author

wrender commented Mar 4, 2024

For some reason when I define these new settings in a cluster crd yaml file, and then deploy a cluster they don't take affect in the container. Am I missing something? For example:

ldapConfiguration
  enabled: true
  tlsTruststore:  /some/path

Then if I exec into the container, and cat conf/login-identity-providers.xml , The value for that line item is empty.

@juldrixx juldrixx requested a review from mh013370 March 4, 2024 22:53
@mh013370
Copy link
Member

mh013370 commented Mar 5, 2024

Just one minor doc suggestion. otherwise it looks good to me

@mh013370
Copy link
Member

mh013370 commented Mar 5, 2024

For some reason when I define these new settings in a cluster crd yaml file, and then deploy a cluster they don't take affect in the container. Am I missing something? For example:

ldapConfiguration
  enabled: true
  tlsTruststore:  /some/path

Then if I exec into the container, and cat conf/login-identity-providers.xml , The value for that line item is empty.

Just double checking you've applied the new CRDs here and the updated operator?

@wrender
Copy link
Contributor Author

wrender commented Mar 5, 2024

For some reason when I define these new settings in a cluster crd yaml file, and then deploy a cluster they don't take affect in the container. Am I missing something? For example:

ldapConfiguration
  enabled: true
  tlsTruststore:  /some/path

Then if I exec into the container, and cat conf/login-identity-providers.xml , The value for that line item is empty.

Just double checking you've applied the new CRDs here and the updated operator?

I deleted the CRDs and re-created them with the helm chart. Maybe I'm not updating the operator correctly? How would I update that?

@mh013370
Copy link
Member

mh013370 commented Mar 5, 2024

For some reason when I define these new settings in a cluster crd yaml file, and then deploy a cluster they don't take affect in the container. Am I missing something? For example:

ldapConfiguration
  enabled: true
  tlsTruststore:  /some/path

Then if I exec into the container, and cat conf/login-identity-providers.xml , The value for that line item is empty.

Just double checking you've applied the new CRDs here and the updated operator?

I deleted the CRDs and re-created them with the helm chart. Maybe I'm not updating the operator correctly? How would I update that?

There's a thread in slack where folks talk about that: https://konpytika.slack.com/archives/C0362VBRM24/p1702911625342399

TLDR is that the helm client doesn't do it for you, but tools like ArgoCD/Flux will do it for you.

https://helm.sh/docs/chart_best_practices/custom_resource_definitions/#some-caveats-and-explanations

Co-authored-by: Michael H <86672176+mh013370@users.noreply.github.com>
@wrender
Copy link
Contributor Author

wrender commented Mar 5, 2024

For some reason when I define these new settings in a cluster crd yaml file, and then deploy a cluster they don't take affect in the container. Am I missing something? For example:

ldapConfiguration
  enabled: true
  tlsTruststore:  /some/path

Then if I exec into the container, and cat conf/login-identity-providers.xml , The value for that line item is empty.

Just double checking you've applied the new CRDs here and the updated operator?

I deleted the CRDs and re-created them with the helm chart. Maybe I'm not updating the operator correctly? How would I update that?

There's a thread in slack where folks talk about that: https://konpytika.slack.com/archives/C0362VBRM24/p1702911625342399

TLDR is that the helm client doesn't do it for you, but tools like ArgoCD/Flux will do it for you.

https://helm.sh/docs/chart_best_practices/custom_resource_definitions/#some-caveats-and-explanations

I manually deleted the CRDs, and confirmed they are updated after re-installing the nifikop helm chart. But still, when I deploy a nifi cluster it doesn't seem to add the ldap settings. Seems like it must be something else then.

@juldrixx
Copy link
Contributor

juldrixx commented Mar 5, 2024

For some reason when I define these new settings in a cluster crd yaml file, and then deploy a cluster they don't take affect in the container. Am I missing something? For example:

ldapConfiguration
  enabled: true
  tlsTruststore:  /some/path

Then if I exec into the container, and cat conf/login-identity-providers.xml , The value for that line item is empty.

Just double checking you've applied the new CRDs here and the updated operator?

I deleted the CRDs and re-created them with the helm chart. Maybe I'm not updating the operator correctly? How would I update that?

There's a thread in slack where folks talk about that: https://konpytika.slack.com/archives/C0362VBRM24/p1702911625342399
TLDR is that the helm client doesn't do it for you, but tools like ArgoCD/Flux will do it for you.
https://helm.sh/docs/chart_best_practices/custom_resource_definitions/#some-caveats-and-explanations

I manually deleted the CRDs, and confirmed they are updated after re-installing the nifikop helm chart. But still, when I deploy a nifi cluster it doesn't seem to add the ldap settings. Seems like it must be something else then.

Did you build an image of the operator with your change? Or are you using the 1.7.0? Or are you running the code locally?

@wrender
Copy link
Contributor Author

wrender commented Mar 5, 2024

For some reason when I define these new settings in a cluster crd yaml file, and then deploy a cluster they don't take affect in the container. Am I missing something? For example:

ldapConfiguration
  enabled: true
  tlsTruststore:  /some/path

Then if I exec into the container, and cat conf/login-identity-providers.xml , The value for that line item is empty.

Just double checking you've applied the new CRDs here and the updated operator?

I deleted the CRDs and re-created them with the helm chart. Maybe I'm not updating the operator correctly? How would I update that?

There's a thread in slack where folks talk about that: https://konpytika.slack.com/archives/C0362VBRM24/p1702911625342399
TLDR is that the helm client doesn't do it for you, but tools like ArgoCD/Flux will do it for you.
https://helm.sh/docs/chart_best_practices/custom_resource_definitions/#some-caveats-and-explanations

I manually deleted the CRDs, and confirmed they are updated after re-installing the nifikop helm chart. But still, when I deploy a nifi cluster it doesn't seem to add the ldap settings. Seems like it must be something else then.

Did you build an image of the operator with your change? Or are you using the 1.7.0? Or are you running the code locally?

Running the code locally. I git cloned my fork, that has the login providers changes for ldap, and then do a helm install of nifikop from that local folder.

@juldrixx
Copy link
Contributor

juldrixx commented Mar 5, 2024

For some reason when I define these new settings in a cluster crd yaml file, and then deploy a cluster they don't take affect in the container. Am I missing something? For example:

ldapConfiguration
  enabled: true
  tlsTruststore:  /some/path

Then if I exec into the container, and cat conf/login-identity-providers.xml , The value for that line item is empty.

Just double checking you've applied the new CRDs here and the updated operator?

I deleted the CRDs and re-created them with the helm chart. Maybe I'm not updating the operator correctly? How would I update that?

There's a thread in slack where folks talk about that: https://konpytika.slack.com/archives/C0362VBRM24/p1702911625342399
TLDR is that the helm client doesn't do it for you, but tools like ArgoCD/Flux will do it for you.
https://helm.sh/docs/chart_best_practices/custom_resource_definitions/#some-caveats-and-explanations

I manually deleted the CRDs, and confirmed they are updated after re-installing the nifikop helm chart. But still, when I deploy a nifi cluster it doesn't seem to add the ldap settings. Seems like it must be something else then.

Did you build an image of the operator with your change? Or are you using the 1.7.0? Or are you running the code locally?

Running the code locally. I git cloned my fork, that has the login providers changes for ldap, and then do a helm install of nifikop from that local folder.

If you didn't build an image of your code to use in your deployment, it won't work. It will just deploy the latest release of the operator. If you want to run the code locally, you need to use a tool like telepresence.

@wrender
Copy link
Contributor Author

wrender commented Mar 5, 2024

For some reason when I define these new settings in a cluster crd yaml file, and then deploy a cluster they don't take affect in the container. Am I missing something? For example:

ldapConfiguration
  enabled: true
  tlsTruststore:  /some/path

Then if I exec into the container, and cat conf/login-identity-providers.xml , The value for that line item is empty.

Just double checking you've applied the new CRDs here and the updated operator?

I deleted the CRDs and re-created them with the helm chart. Maybe I'm not updating the operator correctly? How would I update that?

There's a thread in slack where folks talk about that: https://konpytika.slack.com/archives/C0362VBRM24/p1702911625342399
TLDR is that the helm client doesn't do it for you, but tools like ArgoCD/Flux will do it for you.
https://helm.sh/docs/chart_best_practices/custom_resource_definitions/#some-caveats-and-explanations

I manually deleted the CRDs, and confirmed they are updated after re-installing the nifikop helm chart. But still, when I deploy a nifi cluster it doesn't seem to add the ldap settings. Seems like it must be something else then.

Did you build an image of the operator with your change? Or are you using the 1.7.0? Or are you running the code locally?

Running the code locally. I git cloned my fork, that has the login providers changes for ldap, and then do a helm install of nifikop from that local folder.

If you didn't build an image of your code to use in your deployment, it won't work. It will just deploy the latest release of the operator. If you want to run the code locally, you need to use a tool like telepresence.

Ok. Thanks for the information @juldrixx . I'm new to operators in Kubernetes so I will have to spend some time learning this. Is there any documentation on how I would go about the building of a custom image to test with the deployment? I don't know I want to introduce yet another tool like telepresence.

@juldrixx
Copy link
Contributor

juldrixx commented Mar 5, 2024

For some reason when I define these new settings in a cluster crd yaml file, and then deploy a cluster they don't take affect in the container. Am I missing something? For example:

ldapConfiguration
  enabled: true
  tlsTruststore:  /some/path

Then if I exec into the container, and cat conf/login-identity-providers.xml , The value for that line item is empty.

Just double checking you've applied the new CRDs here and the updated operator?

I deleted the CRDs and re-created them with the helm chart. Maybe I'm not updating the operator correctly? How would I update that?

There's a thread in slack where folks talk about that: https://konpytika.slack.com/archives/C0362VBRM24/p1702911625342399
TLDR is that the helm client doesn't do it for you, but tools like ArgoCD/Flux will do it for you.
https://helm.sh/docs/chart_best_practices/custom_resource_definitions/#some-caveats-and-explanations

I manually deleted the CRDs, and confirmed they are updated after re-installing the nifikop helm chart. But still, when I deploy a nifi cluster it doesn't seem to add the ldap settings. Seems like it must be something else then.

Did you build an image of the operator with your change? Or are you using the 1.7.0? Or are you running the code locally?

Running the code locally. I git cloned my fork, that has the login providers changes for ldap, and then do a helm install of nifikop from that local folder.

If you didn't build an image of your code to use in your deployment, it won't work. It will just deploy the latest release of the operator. If you want to run the code locally, you need to use a tool like telepresence.

Ok. Thanks for the information @juldrixx . I'm new to operators in Kubernetes so I will have to spend some time learning this. Is there any documentation on how I would go about the building of a custom image to test with the deployment? I don't know I want to introduce yet another tool like telepresence.

You can find it here but it doesn't mention telepresence.

@juldrixx juldrixx requested a review from mh013370 March 8, 2024 15:22
@mh013370
Copy link
Member

mh013370 commented Mar 8, 2024

LGTM

@juldrixx juldrixx merged commit 659312a into konpyutaika:master Mar 8, 2024
5 checks passed
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.

None yet

3 participants