Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add more negative test cases for backend config #383

Merged
merged 3 commits into from
Jul 9, 2018
Merged

Add more negative test cases for backend config #383

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3dfcc7f
Unmask get backend config errors
MrHohn Jun 29, 2018
ff38186
Add couple more backend config negative cases
MrHohn Jun 29, 2018
c077184
Actually Create secret for IAP for testing
MrHohn Jul 3, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
143 changes: 98 additions & 45 deletions cmd/e2e-test/backend_config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"k8s.io/apimachinery/pkg/util/wait"

"k8s.io/ingress-gce/pkg/annotations"
backendconfigv1beta1 "k8s.io/ingress-gce/pkg/apis/backendconfig/v1beta1"
"k8s.io/ingress-gce/pkg/e2e"
"k8s.io/ingress-gce/pkg/fuzz"
)
Expand All @@ -37,56 +38,108 @@ var (
eventPollTimeout = 3 * time.Minute
)

func TestBackendConfigNotExist(t *testing.T) {
func TestBackendConfigNegatives(t *testing.T) {
t.Parallel()

Framework.RunWithSandbox("BackendConfig not exist", t, func(t *testing.T, s *e2e.Sandbox) {
testBackendConfigAnnotation := map[string]string{
annotations.BackendConfigKey: `{"default":"not-exist"}`,
}
if _, _, err := e2e.CreateEchoService(s, "service-1", testBackendConfigAnnotation); err != nil {
t.Fatalf("e2e.CreateEchoService(s, service-1, %q) = _, _, %v, want _, _, nil", testBackendConfigAnnotation, err)
}

port80 := intstr.FromInt(80)
testIng := fuzz.NewIngressBuilder("", "ingress-1", "").
AddPath("test.com", "/", "service-1", port80).
Build()
testIng, err := Framework.Clientset.Extensions().Ingresses(s.Namespace).Create(testIng)
if err != nil {
t.Fatalf("error creating Ingress spec: %v", err)
}
t.Logf("Ingress %s/%s created", s.Namespace, testIng.Name)

t.Logf("Waiting for BackendConfig warning event to be emitted")
if err := wait.Poll(eventPollInterval, eventPollTimeout, func() (bool, error) {
events, err := Framework.Clientset.CoreV1().Events(s.Namespace).List(metav1.ListOptions{})
for _, tc := range []struct {
desc string
svcAnnotations map[string]string
backendConfig *backendconfigv1beta1.BackendConfig
secretName string
expectedMsg string
}{
{
desc: "backend config not exist",
svcAnnotations: map[string]string{
annotations.BackendConfigKey: `{"default":"backendconfig-1"}`,
},
expectedMsg: "no BackendConfig",
},
{
desc: "invalid format in backend config annotation",
svcAnnotations: map[string]string{
annotations.BackendConfigKey: `invalid`,
},
expectedMsg: fmt.Sprintf("%v", annotations.ErrBackendConfigInvalidJSON),
},
{
desc: "enable both IAP and CDN in backend config",
svcAnnotations: map[string]string{
annotations.BackendConfigKey: `{"default":"backendconfig-1"}`,
},
backendConfig: fuzz.NewBackendConfigBuilder("", "backendconfig-1").
EnableCDN(true).
SetIAPConfig(true, "bar").
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given how the code is written, you will actually have to create the secret "bar". Otherwise, it will return an error saying the secret does not exist, rather than the error we actually are testing for,

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, I'm seeing this:

Error during sync: error while evaluating the ingress spec: BackendConfig test-sandbox-48912bf33631f0fe/backendconfig-1 is not valid: error retrieving secret bar: secrets \"bar\" not found

Added the logic to actually create the secret "bar".

Build(),
secretName: "bar",
expectedMsg: "iap and cdn cannot be enabled at the same time",
},
} {
tc := tc // Capture tc as we are running this in parallel.
Framework.RunWithSandbox(tc.desc, t, func(t *testing.T, s *e2e.Sandbox) {
t.Parallel()

if tc.backendConfig != nil {
if _, err := Framework.BackendConfigClient.CloudV1beta1().BackendConfigs(s.Namespace).Create(tc.backendConfig); err != nil {
t.Fatalf("Error creating backend config: %v", err)
}
t.Logf("Backend config %s/%s created", s.Namespace, tc.backendConfig.Name)
}

if tc.secretName != "" {
if _, err := e2e.CreateSecret(s, tc.secretName,
map[string][]byte{
"client_id": []byte("my-id"),
"client_secret": []byte("my-secret"),
}); err != nil {
t.Fatalf("Error creating secret %q: %v", tc.secretName, err)
}
}

if _, _, err := e2e.CreateEchoService(s, "service-1", tc.svcAnnotations); err != nil {
t.Fatalf("e2e.CreateEchoService(s, service-1, %q) = _, _, %v, want _, _, nil", tc.svcAnnotations, err)
}

port80 := intstr.FromInt(80)
testIng := fuzz.NewIngressBuilder("", "ingress-1", "").
AddPath("test.com", "/", "service-1", port80).
Build()
testIng, err := Framework.Clientset.Extensions().Ingresses(s.Namespace).Create(testIng)
if err != nil {
return false, fmt.Errorf("error in listing events: %s", err)
t.Fatalf("error creating Ingress spec: %v", err)
}
for _, event := range events.Items {
if event.InvolvedObject.Kind != "Ingress" ||
event.InvolvedObject.Name != "ingress-1" ||
event.Type != v1.EventTypeWarning {
continue
t.Logf("Ingress %s/%s created", s.Namespace, testIng.Name)

t.Logf("Waiting for warning event to be emitted")
if err := wait.Poll(eventPollInterval, eventPollTimeout, func() (bool, error) {
events, err := Framework.Clientset.CoreV1().Events(s.Namespace).List(metav1.ListOptions{})
if err != nil {
return false, fmt.Errorf("error in listing events: %s", err)
}
if strings.Contains(event.Message, "no BackendConfig") {
t.Logf("BackendConfig warning event emitted")
return true, nil
for _, event := range events.Items {
if event.InvolvedObject.Kind != "Ingress" ||
event.InvolvedObject.Name != "ingress-1" ||
event.Type != v1.EventTypeWarning {
continue
}
if strings.Contains(event.Message, tc.expectedMsg) {
t.Logf("Warning event emitted")
return true, nil
}
}
t.Logf("No warning event is emitted yet")
return false, nil
}); err != nil {
t.Fatalf("error waiting for BackendConfig warning event: %v", err)
}

testIng, err = Framework.Clientset.Extensions().Ingresses(s.Namespace).Get(testIng.Name, metav1.GetOptions{})
if err != nil {
t.Fatalf("error retrieving Ingress %q: %v", testIng.Name, err)
}
if len(testIng.Status.LoadBalancer.Ingress) > 0 {
t.Fatalf("Ingress should not have an IP: %+v", testIng.Status)
}
t.Logf("No BackendConfig warning event is emitted yet")
return false, nil
}); err != nil {
t.Fatalf("error waiting for BackendConfig warning event: %v", err)
}

testIng, err = Framework.Clientset.Extensions().Ingresses(s.Namespace).Get(testIng.Name, metav1.GetOptions{})
if err != nil {
t.Fatalf("error retrieving Ingress %q: %v", testIng.Name, err)
}
if len(testIng.Status.LoadBalancer.Ingress) > 0 {
t.Fatalf("Ingress should not have an IP: %+v", testIng.Status)
}
})
})
}
}
9 changes: 4 additions & 5 deletions pkg/backendconfig/backendconfig.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,9 @@ import (
)

var (
ErrBackendConfigsFromAnnotation = errors.New("error getting BackendConfig's from annotation")
ErrBackendConfigDoesNotExist = errors.New("no BackendConfig for service port exists.")
ErrBackendConfigFailedToGet = errors.New("client had error getting BackendConfig for service port.")
ErrNoBackendConfigForPort = errors.New("no BackendConfig name found for service port.")
ErrBackendConfigDoesNotExist = errors.New("no BackendConfig for service port exists.")
ErrBackendConfigFailedToGet = errors.New("client had error getting BackendConfig for service port.")
ErrNoBackendConfigForPort = errors.New("no BackendConfig name found for service port.")
)

func CRDMeta() *crd.CRDMeta {
Expand Down Expand Up @@ -95,7 +94,7 @@ func GetBackendConfigForServicePort(backendConfigLister cache.Store, svc *apiv1.
if err == annotations.ErrBackendConfigAnnotationMissing {
return nil, nil
}
return nil, ErrBackendConfigsFromAnnotation
return nil, err
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What was the reason for the change here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was meant to make the warning event more useful. Without this change, when annotation is in invalid format, user will see something like "error getting backend config" instead of "the annotation is invalid".

}

configName := BackendConfigName(*backendConfigs, *svcPort)
Expand Down