fix(cli): check if installGatewayAPI is true before we error during CRD install (#13872)

This PR incorporates a couple of improvements: 
- we honor the fact that `installGatewayAPI` is true and do not error on missing CRDs
- we take into account `enableHTTPRoutes` when we determine whether CRDs are about to be installed
- tests are refactored and extended for greater coverage

The concrete motivation for revisiting that logic is the fact that the guards introduced
in linkerd/linkerd2#3834  missed to check the situation when
we install with `installGatewayAPI=true` and there are no existing CRDs on the cluster.

Signed-off-by: Zahari Dichev <zaharidichev@gmail.com>
This commit is contained in:
Zahari Dichev 2025-04-01 22:10:35 +03:00 committed by GitHub
parent 2dca1e4606
commit 46b5dcaa8f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 159 additions and 66 deletions

View File

@ -366,6 +366,52 @@ func renderChartToBuffer(files []*loader.BufferedFile, values map[string]interfa
return &buf, vals, nil
}
func updateDefaultValues(installed GatewayAPICRDs, defaultValues map[string]interface{}) map[string]interface{} {
if installed == Absent {
// if GW API is not installed, default to false
defaultValues["installGatewayAPI"] = false
} else if installed == Linkerd {
// if it is installed by Linkerd, default to true
defaultValues["installGatewayAPI"] = true
} else if installed == External {
// if it is external, default to false as we are not managing it
defaultValues["installGatewayAPI"] = false
}
return defaultValues
}
func validateFinalValues(installed GatewayAPICRDs, finalValues map[string]interface{}) error {
installing := false
if installGatewayAPI, ok := finalValues["installGatewayAPI"]; ok {
installing = installGatewayAPI == true
}
if enableHttpRoutes, ok := finalValues["enableHttpRoutes"]; ok {
installing = enableHttpRoutes == true
}
if installed == Absent {
if !installing {
// if we are not installing GW API Resources and they are not present, error
return errors.New("The Gateway API CRDs must be installed prior to installing Linkerd: https://gateway-api.sigs.k8s.io/guides/#installing-gateway-api")
}
} else if installed == Linkerd {
if !installing {
// if they are installed and managed by Linkerd, we cannot uninstall them
return errors.New("Linkerd is providing GW API, but your current install configuration will remove it")
}
} else if installed == External {
if installing {
// if they are installed but are external, we cannot be installing as well
return errors.New("Linkerd cannot install the Gateway API CRDs because they are already installed by an external source. Please set `installGatewayAPI` to `false`.")
}
}
return nil
}
func renderCRDs(ctx context.Context, k *k8s.KubernetesAPI, w io.Writer, options valuespkg.Options, format string) error {
files := []*loader.BufferedFile{
{Name: chartutil.ChartfileName},
@ -406,17 +452,15 @@ func renderCRDs(ctx context.Context, k *k8s.KubernetesAPI, w io.Writer, options
if err != nil {
return err
}
if installed == Absent {
return errors.New("The Gateway API CRDs must be installed prior to installing Linkerd: https://gateway-api.sigs.k8s.io/guides/#installing-gateway-api")
} else if installed == Linkerd {
defaultValues["installGatewayAPI"] = true
} else if installed == External {
defaultValues["installGatewayAPI"] = false
if valuesOverrides["installGatewayAPI"] == true {
return errors.New("Linkerd cannot install the Gateway API CRDs because they are already installed by an external source. Please set `installGatewayAPI` to `false`.")
}
defaultValues = updateDefaultValues(installed, defaultValues)
finalValues := charts.MergeMaps(defaultValues, valuesOverrides)
if err := validateFinalValues(installed, finalValues); err != nil {
return err
}
}
buf, _, err := renderChartToBuffer(files, defaultValues, valuesOverrides)
if err != nil {
return err

View File

@ -294,78 +294,127 @@ func TestIgnoreCluster(t *testing.T) {
}
}
func TestRenderCRDsWithExternalGatewayAPI(t *testing.T) {
gatewayAPIManifest := `---
func TestGWApi(t *testing.T) {
externalGatewayAPIManifest := `---
apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
name: httproutes.gateway.networking.k8s.io
`
k, err := k8s.NewFakeAPIFromManifests([]io.Reader{strings.NewReader(gatewayAPIManifest)})
if err != nil {
t.Fatalf("failed to initialize fake API: %s", err)
}
var buf bytes.Buffer
if err := renderCRDs(context.Background(), k, &buf, values.Options{}, "yaml"); err != nil {
t.Fatalf("Failed to render templates: %v", err)
}
if err := testDataDiffer.DiffTestYAML("install_crds.golden", buf.String()); err != nil {
t.Error(err)
}
}
func TestRenderCRDsWithMissingGatewayAPI(t *testing.T) {
k, err := k8s.NewFakeAPIFromManifests([]io.Reader{})
if err != nil {
t.Fatalf("failed to initialize fake API: %s", err)
}
var buf bytes.Buffer
err = renderCRDs(context.Background(), k, &buf, values.Options{}, "yaml")
if err == nil {
t.Fatalf("Installing with missing Gateway API CRDs should fail")
}
}
func TestRenderCRDsWithGatewayAPI(t *testing.T) {
options := values.Options{
Values: []string{"installGatewayAPI=true"},
}
var buf bytes.Buffer
if err := renderCRDs(context.Background(), nil, &buf, options, "yaml"); err != nil {
t.Fatalf("Failed to render templates: %v", err)
}
if err := testDataDiffer.DiffTestYAML("install_crds_with_gateway_api.golden", buf.String()); err != nil {
t.Error(err)
}
}
func TestRenderCRDsWithConflictingGatewayAPI(t *testing.T) {
gatewayAPIManifest := `---
linkerdGatewayAPIManifest := `---
apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
name: httproutes.gateway.networking.k8s.io
annotations:
linkerd.io/created-by: linkerd
`
k, err := k8s.NewFakeAPIFromManifests([]io.Reader{strings.NewReader(gatewayAPIManifest)})
if err != nil {
t.Fatalf("failed to initialize fake API: %s", err)
testCases := []struct {
name string
resources string
values values.Options
expectError bool
goldenFile string
}{
{
"render with external GW API",
externalGatewayAPIManifest,
values.Options{},
false,
"install_crds.golden",
},
{
"render with missing GW API",
"",
values.Options{},
true,
"",
},
{
"render with GW API (installGatewayAPI flag)",
"",
values.Options{
Values: []string{"installGatewayAPI=true"},
},
false,
"install_crds_with_gateway_api.golden",
},
{
"render with GW API (lagecy flags)",
"",
values.Options{
Values: []string{
"enableHttpRoutes=true",
"enableTlsRoutes=true",
"enableTcpRoutes=true",
},
},
false,
"install_crds_with_gateway_api.golden",
},
{
"render with conflicting GW API (installGatewayAPI flag)",
externalGatewayAPIManifest,
values.Options{
Values: []string{"installGatewayAPI=true"},
},
true,
"",
},
{
"render with conflicting GW API (legacy flags)",
externalGatewayAPIManifest,
values.Options{
Values: []string{
"enableHttpRoutes=true",
"enableTlsRoutes=true",
"enableTcpRoutes=true",
},
},
true,
"",
},
{
"error on attempt to remove a Linkerd managed version via installGatewayAPI=false",
linkerdGatewayAPIManifest,
values.Options{
Values: []string{"installGatewayAPI=false"},
},
true,
"",
},
}
options := values.Options{
Values: []string{"installGatewayAPI=true"},
}
var buf bytes.Buffer
err = renderCRDs(context.Background(), k, &buf, options, "yaml")
if err == nil {
t.Fatalf("Installing with conflicting Gateway API CRDs should fail")
for _, tc := range testCases {
tc := tc // pin
t.Run(tc.name, func(t *testing.T) {
k, err := k8s.NewFakeAPIFromManifests([]io.Reader{strings.NewReader(tc.resources)})
if err != nil {
t.Fatalf("failed to initialize fake API: %s", err)
}
var buf bytes.Buffer
err = renderCRDs(context.Background(), k, &buf, tc.values, "yaml")
if err != nil {
if tc.expectError {
return
}
t.Fatalf("Failed to render templates: %v", err)
}
if tc.expectError && err == nil {
t.Fatal("an error was expected")
}
if err := testDataDiffer.DiffTestYAML(tc.goldenFile, buf.String()); err != nil {
t.Error(err)
}
})
}
}
func TestValidateAndBuild_Errors(t *testing.T) {