infinispan / infinispan-helm-charts

Apache License 2.0
14 stars 29 forks source link

issue #22 deployment config #29

Closed domiborges closed 2 years ago

domiborges commented 2 years ago

@oraNod few questions A values file with all the configuration values or something lighter? WDYT? If the changes are OK I'd update the dowstream content in a similar way. I just wanted to check it with you first. I'm looking for a better place to include the reference module with the server configuration. Cache Manager TBD

oraNod commented 2 years ago

@ryanemerson I've reworked this to copy the default server configuration for the deploy.infinispan section. This was the original plan and then point to the server guide for specific examples. I'd prefer to keep that approach as it will avoid us having too many config snippets in the helm chart docs repo, which presents a maintenance headache. Please take another look when you get a chance.

@dvagnero If the server configuration examples are OK, and they should be, I think all that is left with this PR is to make sure the procedure to disable authorization is correct. Ryan is going to make some changes tomorrow to enable security authz by default so users will need to know how to disable it. I did add a procedure but we should review it with Ryan after he makes the change tomorrow. We can then squash the commits and merge. :wind_chime:

ryanemerson commented 2 years ago

@oraNod It turns out that I was telling you wrong ... Authorization is currently enabled by default. In order to disable authorization you need to set deploy.infinispan.cacheContainer.security=null whereas the default config sets ...security.authorization=null.

Even so, I will update values.yaml to use an empty element:

  infinispan:
    cacheContainer:
      security:
        authorization: {}

As that's slightly more intuitive than null.

ryanemerson commented 2 years ago

It's looking good @oraNod @dvagnero!

Can you update the values.yaml in the repo to be the same as the ones in the docs with the [USER] comments?

Just a thought on structure. Should we move section 1.6 to be 2.0 and call this something like "Infinispan Server Configuration"? To me section 1.6 seems a separate topic to the other 1.x sections that are more related to chart lifecycle and initialisation. "Configuring authentication and authorization" could then be a subsection of the server configuration, WDYT?

ryanemerson commented 2 years ago

Thanks @dvagnero and @oraNod