diff --git a/internal/adc/translator/consumer.go b/internal/adc/translator/consumer.go index d7e638be..d055725a 100644 --- a/internal/adc/translator/consumer.go +++ b/internal/adc/translator/consumer.go @@ -19,6 +19,7 @@ package translator import ( "encoding/json" + "fmt" "k8s.io/apimachinery/pkg/types" @@ -63,11 +64,7 @@ func (t *Translator) TranslateConsumerV1alpha1(tctx *provider.TranslateContext, } else { authConfig := make(map[string]any) if err := json.Unmarshal(credentialSpec.Config.Raw, &authConfig); err != nil { - t.Log.Error(err, "failed to unmarshal credential config", - "consumer", consumerV.Namespace+"/"+consumerV.Name, - "credential", credentialSpec.Name, - "type", credentialSpec.Type) - continue + return nil, fmt.Errorf("consumer %s/%s credential %s: failed to unmarshal config: %w", consumerV.Namespace, consumerV.Name, credentialSpec.Name, err) } credential.Config = authConfig } @@ -79,10 +76,7 @@ func (t *Translator) TranslateConsumerV1alpha1(tctx *provider.TranslateContext, for _, plugin := range consumerV.Spec.Plugins { pluginConfig, err := renderPluginConfig(plugin, consumerV.Namespace, tctx.Secrets) if err != nil { - t.Log.Error(err, "failed to render plugin config", - "consumer", consumerV.Namespace+"/"+consumerV.Name, - "plugin", plugin.Name) - continue + return nil, fmt.Errorf("consumer %s/%s plugin %s: %w", consumerV.Namespace, consumerV.Name, plugin.Name, err) } plugins[plugin.Name] = pluginConfig } diff --git a/internal/adc/translator/consumer_test.go b/internal/adc/translator/consumer_test.go index 6f5d08ce..619dfa9d 100644 --- a/internal/adc/translator/consumer_test.go +++ b/internal/adc/translator/consumer_test.go @@ -23,7 +23,10 @@ import ( "github.com/go-logr/logr" "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" "github.com/apache/apisix-ingress-controller/api/v1alpha1" "github.com/apache/apisix-ingress-controller/internal/controller/label" @@ -59,3 +62,47 @@ func TestTranslateConsumerV1alpha1_UsesMetadataLabelsWithoutOverwritingControlle require.Equal(t, consumer.Name, translated.Labels[label.LabelName]) require.Equal(t, "Consumer/default/demo", translated.Labels[label.LabelResourceKey]) } + +func TestTranslateConsumerRejectsIncompleteConfiguration(t *testing.T) { + for _, tc := range []struct { + name string + credentials []v1alpha1.Credential + plugins []v1alpha1.Plugin + want string + }{ + {name: "malformed credential", credentials: []v1alpha1.Credential{{Name: "bad", Type: "key-auth", Config: apiextensionsv1.JSON{Raw: []byte(`{"key":`)}}}, want: "credential bad"}, + {name: "non-object credential", credentials: []v1alpha1.Credential{{Name: "bad", Type: "key-auth", Config: apiextensionsv1.JSON{Raw: []byte(`[]`)}}}, want: "credential bad"}, + {name: "malformed plugin", plugins: []v1alpha1.Plugin{{Name: "limit-count", Config: apiextensionsv1.JSON{Raw: []byte(`{"count":`)}}}, want: "plugin limit-count"}, + {name: "non-object plugin", plugins: []v1alpha1.Plugin{{Name: "limit-count", Config: apiextensionsv1.JSON{Raw: []byte(`[]`)}}}, want: "plugin limit-count"}, + {name: "missing plugin secret", plugins: []v1alpha1.Plugin{{Name: "limit-count", SecretRef: &corev1.LocalObjectReference{Name: "missing"}}}, want: "secret default/missing"}, + } { + t.Run(tc.name, func(t *testing.T) { + consumer := &v1alpha1.Consumer{ObjectMeta: metav1.ObjectMeta{Name: "demo", Namespace: "default"}} + consumer.Spec.Credentials = append([]v1alpha1.Credential{{Name: "valid", Type: "key-auth", Config: apiextensionsv1.JSON{Raw: []byte(`{"key":"test-key"}`)}}}, tc.credentials...) + consumer.Spec.Plugins = append([]v1alpha1.Plugin{{Name: "response-rewrite", Config: apiextensionsv1.JSON{Raw: []byte(`{"status_code":403}`)}}}, tc.plugins...) + result, err := NewTranslator(logr.Discard(), "").TranslateConsumerV1alpha1(provider.NewDefaultTranslateContext(context.Background()), consumer) + require.ErrorContains(t, err, tc.want) + require.ErrorContains(t, err, "consumer default/demo") + require.Nil(t, result, "must not publish a partial consumer") + require.NotContains(t, err.Error(), "test-key") + }) + } +} + +func TestTranslateConsumerPreservesValidConfiguration(t *testing.T) { + ctx := provider.NewDefaultTranslateContext(context.Background()) + ctx.Secrets[types.NamespacedName{Namespace: "default", Name: "credential"}] = &corev1.Secret{Data: map[string][]byte{"key": []byte("secret-key")}} + consumer := &v1alpha1.Consumer{ObjectMeta: metav1.ObjectMeta{Name: "demo", Namespace: "default"}} + consumer.Spec.Credentials = []v1alpha1.Credential{ + {Name: "inline", Type: "key-auth", Config: apiextensionsv1.JSON{Raw: []byte(`{"key":"inline-key"}`)}}, + {Name: "secret", Type: "key-auth", SecretRef: &v1alpha1.SecretReference{Name: "credential"}}, + } + consumer.Spec.Plugins = []v1alpha1.Plugin{{Name: "response-rewrite", Config: apiextensionsv1.JSON{Raw: []byte(`{"status_code":403}`)}}} + result, err := NewTranslator(logr.Discard(), "").TranslateConsumerV1alpha1(ctx, consumer) + require.NoError(t, err) + require.Len(t, result.Consumers, 1) + require.Len(t, result.Consumers[0].Credentials, 2) + require.Equal(t, "inline-key", result.Consumers[0].Credentials[0].Config["key"]) + require.Equal(t, "secret-key", result.Consumers[0].Credentials[1].Config["key"]) + require.Equal(t, float64(403), result.Consumers[0].Plugins["response-rewrite"].(map[string]any)["status_code"]) +} diff --git a/internal/provider/api7ee/provider_test.go b/internal/provider/api7ee/provider_test.go index 8ef49365..e9603e17 100644 --- a/internal/provider/api7ee/provider_test.go +++ b/internal/provider/api7ee/provider_test.go @@ -133,3 +133,29 @@ func TestUpdateKeepsLastKnownGoodStateWhenL4PolicyCannotRender(t *testing.T) { require.Len(t, resources.Services, 1) assert.Equal(t, "last-known-good", resources.Services[0].Name) } + +func TestUpdateRejectsInvalidConsumerConfiguration(t *testing.T) { + for _, plugin := range []bool{false, true} { + name := "credential" + if plugin { + name = "plugin" + } + t.Run(name, func(t *testing.T) { + raw, err := New(logr.Discard(), nil, nil) + require.NoError(t, err) + consumer := &v1alpha1.Consumer{ + TypeMeta: metav1.TypeMeta{Kind: "Consumer", APIVersion: v1alpha1.GroupVersion.String()}, + ObjectMeta: metav1.ObjectMeta{Namespace: "default", Name: "demo"}, + } + if plugin { + consumer.Spec.Plugins = []v1alpha1.Plugin{{Name: "ip-restriction", Config: apiextensionsv1.JSON{Raw: []byte(`[]`)}}} + } else { + consumer.Spec.Credentials = []v1alpha1.Credential{{Name: "invalid", Type: "key-auth", Config: apiextensionsv1.JSON{Raw: []byte(`[]`)}}} + } + // Translation errors must propagate before provider configuration or submission. + err = raw.Update(context.Background(), provider.NewDefaultTranslateContext(context.Background()), consumer) + require.ErrorContains(t, err, "consumer default/demo") + require.ErrorContains(t, err, name) + }) + } +} diff --git a/internal/provider/apisix/provider_test.go b/internal/provider/apisix/provider_test.go index 50d9e420..c0153492 100644 --- a/internal/provider/apisix/provider_test.go +++ b/internal/provider/apisix/provider_test.go @@ -170,3 +170,29 @@ func TestUpdateKeepsLastKnownGoodStateWhenL4PolicyCannotRender(t *testing.T) { require.Len(t, resources.Services, 1) assert.Equal(t, "last-known-good", resources.Services[0].Name) } + +func TestUpdateRejectsInvalidConsumerConfiguration(t *testing.T) { + for _, plugin := range []bool{false, true} { + name := "credential" + if plugin { + name = "plugin" + } + t.Run(name, func(t *testing.T) { + raw, err := New(logr.Discard(), nil, nil) + require.NoError(t, err) + consumer := &v1alpha1.Consumer{ + TypeMeta: metav1.TypeMeta{Kind: "Consumer", APIVersion: v1alpha1.GroupVersion.String()}, + ObjectMeta: metav1.ObjectMeta{Namespace: "default", Name: "demo"}, + } + if plugin { + consumer.Spec.Plugins = []v1alpha1.Plugin{{Name: "ip-restriction", Config: apiextensionsv1.JSON{Raw: []byte(`[]`)}}} + } else { + consumer.Spec.Credentials = []v1alpha1.Credential{{Name: "invalid", Type: "key-auth", Config: apiextensionsv1.JSON{Raw: []byte(`[]`)}}} + } + // Translation errors must propagate before provider configuration or submission. + err = raw.Update(context.Background(), provider.NewDefaultTranslateContext(context.Background()), consumer) + require.ErrorContains(t, err, "consumer default/demo") + require.ErrorContains(t, err, name) + }) + } +}