Remove --auth-anonymous if kube_api_anonymous_auth is undefined

[psychomantys]

What type of PR is this?

/kind feature

What this PR does / why we need it:

This PR introduces changes to address the incompatibility between the kube_api_anonymous_auth flag and the --authentication-config argument in kubeadm. The current implementation adds --anonymous-auth=true|false based on the value of kube_api_anonymous_auth, which is not compatible with the --authentication-config configuration file, when the config file includes a configuration for anonymous authentication.

This PR modifies the behavior to remove the --anonymous-auth flag when kube_api_anonymous_auth is "{{ undef() }}", ensuring compatibility without conflicting configurations.

Which issue(s) this PR fixes:

Additionally, if kube-apiserver uses --anonynous-auth=true|false and --authorization-config with a configuration like the following:

apiVersion: apiserver.config.k8s.io/v1beta1
kind: AuthenticationConfiguration
anonymous:
  enabled: true
  conditions:
    - path: /livez
    - path: /readyz
    - path: /healthz
    - path: /api/v1/namespaces/kube-public/configmaps/cluster-info
jwt:
  - issuer:
      url: https://false.url/realms/myrealm
      audiences:
        - verylongaud
    claimMappings:
      username:
        claim: email
        prefix: "oidc:"
      groups:
        claim: groups
        prefix: "oidc:"
    userValidationRules:
      - expression: "!user.username.startsWith('system:')"
        message: "username cannot use reserved system: prefix"
      - expression: "user.groups.all(group, !group.startsWith('system:'))"
        message: 'groups cannot be used with reserved system: prefix'

An error log message will exist in kube-apiserver:

E0628 08:00:07.762676       1 run.go:72] "command failed" err="anonymous: Forbidden: --anonynous-auth flag cannot be set when anonymous 
field is configured in authentication configuration file"

As describe in the KEP-4633 and in documentation.

Special notes for your reviewer:

Maybe this issue was introduced by #12307. It would be nice to port the fix to older versions.

Does this PR introduce a user-facing change?:

Remove --auth-anonymous if kube_api_anonymous_auth is undefined.

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: psychomantys / name: Psycho Mantys (85b7490)

[k8s-ci-robot]

Welcome @psychomantys!

It looks like this is your first PR to kubernetes-sigs/kubespray 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/kubespray has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

[k8s-ci-robot]

Hi @psychomantys. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[VannTen]

Hum, do you mean that the anonymous-auth flag is actually a tristate rather than a boolean (aka undefined/true/false), and that only undefined is compatible with using structured authentication (or only the anonymous in structured authentication). I'm not a big fan of negative flags because they lead to double negatives which are not great for comprehension, maybe there is another alternative, like not defining the flag ?

[psychomantys]

@VannTen Yes, the --anonymous-auth flag is considered tristate, as i test with true and false in the latest version, showing the log in PR. I find the documentation how say that.

Initially, I considered moving to an undefined state, similar to the pull request that removed this behavior. However, for greater compatibility and because the variable already had a defined default value, I decided to add the variable instead. Since I perceive indications of potential larger changes, I will modify the pull request accordingly.

[psychomantys]

@VannTen I've updated the branch to remove argument --anonymous-auth if kube_api_anonymous_auth is "{{ undef() }}".

Please review if this meets your requirements or if any further adjustments are needed. Feel free to reach out for any clarifications or changes.

[VannTen]

/ok-to-test

[VannTen]

/retest

Hum, I was thinking we could check the authentication config variable but actually #11841 is not merged 🤔

I think your approach is acceptable, and once we support creating the structured authentication we'll be able to inspect it and remove the flag depending on the presence of the anonymous key in the top level.

Could you add a TODO: note (on the ifdef) to rework once suppport for structured auth lands ?
Thanks.

[psychomantys]

@VannTen Yes, I can make them work and be compatible. I will make the adjustments and then let you know.

[psychomantys]

@VannTen I decided to make a condition to test the variable kube_apiserver_use_authentication_config_file, but realizing that it's possible to use both by checking the documentation, which explicitly states you can use --anonymous-auth and AuthenticationConfiguration, as long as AuthenticationConfiguration.Anonymous is null.

Therefore, to achieve this, there would be one more a state in the kube_api_anonymous_auth variable that could be a dictionary containing the AuthenticationConfiguration configuration for anonymous auth. Furthermore, since PR #11841 has not already been merged to also change the AuthenticationConfiguration file, for simplicity's sake, I decided to revert back to my original idea and just put TODO for future consideration.

Feel free to reach out for anything more.

[VannTen]

Yeah let's keep it simple for now, I didn't expect you to integrate with an unmerged PR 👍

Thanks for the work
/approve
Could you remove the merge commit ? I think a rebase on master should do.

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: psychomantys, VannTen

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [VannTen]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[tico88612]

@psychomantys, please remove the merge commit and use rebase.

[psychomantys]

@tico88612 Sorry for the mess, I had forgotten about it. Everything is fine now.

[VannTen]

Hum, I think you still need to rebase ? The missing files in tests/ are usually a symptom of changed stuff in the tip of the target branch.