Flag error during conversion of `StructuredAuthentication` as user error

[RadaBDimitrova]

How to categorize this PR?

/area ops-productivity /kind enhancement

What this PR does / why we need it: When met with the following error, it sounds like a pretty obvious user error in the configuration:

Last Error
task "Deploying Kubernetes API server" failed: retry failed with context deadline exceeded, last error: converting (v1alpha1.AuthenticationConfiguration) to (v1beta1.AuthenticationConfiguration): unknown conversion

This PR aims to mark the error as a user error.

How to reproduce locally: Apply this breaking ConfigMap:

apiVersion: v1
data:
  config.yaml: |
    apiVersion: apiserver.config.k8s.io/v1alpha1
    kind: AuthenticationConfiguration
kind: ConfigMap
metadata:
  name: structured-auth-config
  namespace: garden-local

Then apply provider-local shoot with reference to ConfigMap:

...
    kubeAPIServer:
      structuredAuthentication:
        configMapName: structured-auth-config

Then it should be marked as a user error. Which issue(s) this PR fixes: N/A

Special notes for your reviewer: /cc @dimityrmirchev @AleksandarSavchev @Kostov6

Release note:

NONE

[gardener-prow[bot]]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Once this PR has been reviewed and has the lgtm label, please assign timuthy for approval. For more information see the Code Review Process.

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

Needs approval from an approver in each of these files:

• OWNERS

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

[gardener-prow[bot]]

@RadaBDimitrova: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-gardener-integration	a0624d01922bf449a6b6487ee1ee6908da0965fd	link	true	/test pull-gardener-integration
pull-gardener-e2e-kind-gardenadm	a0624d01922bf449a6b6487ee1ee6908da0965fd	link	true	/test pull-gardener-e2e-kind-gardenadm

Full PR test history. Your PR dashboard. Command help for this repository. Please help us cut down on flakes by linking this test failure to an open flake report or filing a new flake report if you can't find an existing one. Also see our testing guideline for how to avoid and hunt flakes.

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. I understand the commands that are listed here.

[AleksandarSavchev]

We allow admission of AuthenticationConfiguration resource of API Version apiserver.config.k8s.io/v1alpha1 ref.

This case looks like a regression from https://github.com/gardener/gardener/pull/12198, where now the AuthenticationConfiguration is decoded into a struct of APIVersion=apiserver.config.k8s.io/v1beta1. We already have validation for the referenced authenticationConfig ref ~~and should be able to simply use the string without decoding it~~. I do not think there is a reason not to support API Version v1alpha1.

[Kostov6]

In that case how do you suggest we handle the highlighted case?

[AleksandarSavchev]

Given that we also want to modify the AuthenticationConfiguration here, we should add cases for both versions. We should also stick to v1beta1 version in the case where the Shoot owner has not provided the configuration.

[ialidzhikov]

ping @dimityrmirchev as it is a regression introduced with https://github.com/gardener/gardener/pull/12198

[dimityrmirchev]

Can we do something like this?

diff --git a/pkg/component/kubernetes/apiserver/authentication.go b/pkg/component/kubernetes/apiserver/authentication.go
index 2168090452..5fbcf918d9 100644
--- a/pkg/component/kubernetes/apiserver/authentication.go
+++ b/pkg/component/kubernetes/apiserver/authentication.go
@@ -16,6 +16,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
+	"k8s.io/apiserver/pkg/apis/apiserver"
 	apiserverv1beta1 "k8s.io/apiserver/pkg/apis/apiserver/v1beta1"
 	"k8s.io/utils/ptr"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -52,7 +53,7 @@ func (k *kubeAPIServer) reconcileConfigMapAuthenticationConfig(ctx context.Conte
 
 	authenticationConfig := ptr.Deref(k.values.AuthenticationConfiguration, "")
 
-	authenticationConfiguration := &apiserverv1beta1.AuthenticationConfiguration{
+	authenticationConfigurationV1Beta1 := &apiserverv1beta1.AuthenticationConfiguration{
 		TypeMeta: metav1.TypeMeta{
 			APIVersion: apiserverv1beta1.ConfigSchemeGroupVersion.String(),
 			Kind:       "AuthenticationConfiguration",
@@ -60,22 +61,26 @@ func (k *kubeAPIServer) reconcileConfigMapAuthenticationConfig(ctx context.Conte
 	}
 
 	if len(authenticationConfig) > 0 {
+		authenticationConfiguration := &apiserver.AuthenticationConfiguration{}
 		if err := runtime.DecodeInto(ConfigCodec, []byte(authenticationConfig), authenticationConfiguration); err != nil {
 			return err
 		}
+		if err := apiserverv1beta1.Convert_apiserver_AuthenticationConfiguration_To_v1beta1_AuthenticationConfiguration(authenticationConfiguration, authenticationConfigurationV1Beta1, nil); err != nil {
+			return err
+		}
 	}
 
 	if k.values.OIDC != nil {
-		authenticationConfiguration.JWT = ConfigureJWTAuthenticators(k.values.OIDC)
+		authenticationConfigurationV1Beta1.JWT = ConfigureJWTAuthenticators(k.values.OIDC)
 	}
 
-	if k.anonymousAuthConfigurableEndpointsFeatureGateEnabled() && authenticationConfiguration.Anonymous == nil {
-		authenticationConfiguration.Anonymous = &apiserverv1beta1.AnonymousAuthConfig{
+	if k.anonymousAuthConfigurableEndpointsFeatureGateEnabled() && authenticationConfigurationV1Beta1.Anonymous == nil {
+		authenticationConfigurationV1Beta1.Anonymous = &apiserverv1beta1.AnonymousAuthConfig{
 			Enabled: ptr.Deref(k.values.AnonymousAuthenticationEnabled, false),
 		}
 	}
 
-	data, err := runtime.Encode(ConfigCodec, authenticationConfiguration)
+	data, err := runtime.Encode(ConfigCodec, authenticationConfigurationV1Beta1)
 	if err != nil {
 		return fmt.Errorf("unable to encode authentication configuration: %w", err)
 	}

I was looking for a direct conversion function as @ialidzhikov suggested in a private conversation, but only found scheme.Convert which does not seem to simplify code.

WDYT?

[ialidzhikov]

I was looking for a direct conversion function as @ialidzhikov suggested in a private conversation, but only found scheme.Convert which does not seem to simplify code.

Can't we make it work without explicit conversion? I would assume that the decoder should be able to convert v1alpha -> internal -> v1beta1 when you have a v1alpha1 version and you want to decode it as v1beta1? Or, am I totally wrong?

[ialidzhikov]

Can't we make it work without explicit conversion? I would assume that the decoder should be able to convert v1alpha -> internal -> v1beta1 when you have a v1alpha1 version and you want to decode it as v1beta1? Or, am I totally wrong?

@dimityrmirchev, can this work?

[dimityrmirchev]

Can't we make it work without explicit conversion? I would assume that the decoder should be able to convert v1alpha -> internal -> v1beta1 when you have a v1alpha1 version and you want to decode it as v1beta1? Or, am I totally wrong?

@dimityrmirchev, can this work?

I did not find a way to do it.

Here is my reproducer which still yields and error converting (v1alpha1.AuthenticationConfiguration) to (v1beta1.AuthenticationConfiguration): unknown conversion

package main

import (
	"log"

	"k8s.io/apimachinery/pkg/runtime"
	"k8s.io/apimachinery/pkg/runtime/schema"
	"k8s.io/apimachinery/pkg/runtime/serializer"
	"k8s.io/apimachinery/pkg/runtime/serializer/json"
	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
	"k8s.io/apiserver/pkg/apis/apiserver"
	apiserverv1 "k8s.io/apiserver/pkg/apis/apiserver/v1"
	apiserverv1alpha1 "k8s.io/apiserver/pkg/apis/apiserver/v1alpha1"
	apiserverv1beta1 "k8s.io/apiserver/pkg/apis/apiserver/v1beta1"
)

var ConfigCodec runtime.Codec

func init() {
	scheme := runtime.NewScheme()
	utilruntime.Must(apiserverv1alpha1.AddToScheme(scheme))
	utilruntime.Must(apiserverv1beta1.AddToScheme(scheme))
	utilruntime.Must(apiserverv1.AddToScheme(scheme))
	utilruntime.Must(apiserver.AddToScheme(scheme))

	var (
		ser = json.NewSerializerWithOptions(json.DefaultMetaFactory, scheme, scheme, json.SerializerOptions{
			Yaml:   true,
			Pretty: false,
			Strict: false,
		})
		versions = schema.GroupVersions([]schema.GroupVersion{
			apiserver.SchemeGroupVersion,
			apiserverv1alpha1.SchemeGroupVersion,
			apiserverv1alpha1.ConfigSchemeGroupVersion,
			apiserverv1beta1.SchemeGroupVersion,
			apiserverv1beta1.ConfigSchemeGroupVersion,
			apiserverv1.SchemeGroupVersion,
		})
	)

	ConfigCodec = serializer.NewCodecFactory(scheme).CodecForVersions(ser, ser, versions, versions)
}

var authAconfig = `
apiVersion: apiserver.config.k8s.io/v1alpha1
kind: AuthenticationConfiguration
jwt:
- issuer:
    url: https://foo.com
    audiences:
    - example-client-id
  claimMappings:
    username:
      claim: username
      prefix: "foo:"
`

func main() {
	authenticationConfiguration := &apiserverv1beta1.AuthenticationConfiguration{}

	if err := runtime.DecodeInto(ConfigCodec, []byte(authAconfig), authenticationConfiguration); err != nil {
		log.Fatal(err)
	}
}

[ialidzhikov]

@dimityrmirchev thanks for trying out and proposing a diff with a fix that would work. Let's then proceed with your suggestion in https://github.com/gardener/gardener/pull/12365#issuecomment-2995428085 and include this PR in v1.122. We also have to cherry-pick to release-v1.121 branch.

[Kostov6]

As the conversion between beta <-> alpha is not 1:1 and after talking to @AleksandarSavchev, code authors will take over the fix