diff --git a/pkg/loadbalancers/loadbalancers_test.go b/pkg/loadbalancers/loadbalancers_test.go index a3f557800a..84174e07c7 100644 --- a/pkg/loadbalancers/loadbalancers_test.go +++ b/pkg/loadbalancers/loadbalancers_test.go @@ -20,6 +20,7 @@ import ( "context" "fmt" "net/http" + "reflect" "strconv" "strings" "testing" @@ -34,6 +35,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/ingress-gce/pkg/annotations" + frontendconfigv1beta1 "k8s.io/ingress-gce/pkg/apis/frontendconfig/v1beta1" "k8s.io/ingress-gce/pkg/composite" "k8s.io/ingress-gce/pkg/events" "k8s.io/ingress-gce/pkg/flags" @@ -86,7 +88,7 @@ func newTestJig(t *testing.T) *testJig { if err != nil { return &googleapi.Error{ Code: http.StatusNotFound, - Message: fmt.Sprintf("Key: %s was not found in UrlMaps", key.String()), + Message: fmt.Sprintf("Key: %s was not found in TargetHttpsProxies", key.String()), } } @@ -97,6 +99,18 @@ func newTestJig(t *testing.T) *testJig { tp.SslCertificates = request.SslCertificates return nil } + mockGCE.MockTargetHttpsProxies.SetSslPolicyHook = func(ctx context.Context, key *meta.Key, ref *compute.SslPolicyReference, proxies *cloud.MockTargetHttpsProxies) error { + tps, err := proxies.Get(ctx, key) + fmt.Printf("tps = %+v, err = %v\n\n", tps, err) + if err != nil { + return &googleapi.Error{ + Code: http.StatusNotFound, + Message: fmt.Sprintf("Key: %s was not found in TargetHttpsProxies", key.String()), + } + } + tps.SslPolicy = ref.SslPolicy + return nil + } mockGCE.MockGlobalForwardingRules.InsertHook = InsertGlobalForwardingRuleHook ing := newIngress() @@ -973,6 +987,159 @@ func TestCreateBothLoadBalancers(t *testing.T) { } } +// Test setting frontendconfig Ssl policy +func TestFrontendConfigSslPolicy(t *testing.T) { + flags.F.EnableFrontendConfig = true + defer func() { flags.F.EnableFrontendConfig = false }() + + j := newTestJig(t) + + gceUrlMap := utils.NewGCEURLMap() + gceUrlMap.DefaultBackend = &utils.ServicePort{NodePort: 31234, BackendNamer: j.namer} + gceUrlMap.PutPathRulesForHost("bar.example.com", []utils.PathRule{{Path: "/bar", Backend: utils.ServicePort{NodePort: 30000, BackendNamer: j.namer}}}) + lbInfo := &L7RuntimeInfo{ + AllowHTTP: false, + TLS: []*TLSCerts{createCert("key", "cert", "name")}, + UrlMap: gceUrlMap, + Ingress: newIngress(), + FrontendConfig: &frontendconfigv1beta1.FrontendConfig{Spec: frontendconfigv1beta1.FrontendConfigSpec{SslPolicy: utils.NewStringPointer("test-policy")}}, + } + + l7, err := j.pool.Ensure(lbInfo) + if err != nil { + t.Fatalf("j.pool.Ensure(%v) = %v, want nil", lbInfo, err) + } + + tpsName := l7.tps.Name + tps, _ := composite.GetTargetHttpsProxy(j.fakeGCE, meta.GlobalKey(tpsName), meta.VersionGA) + + resourceID, err := cloud.ParseResourceURL(tps.SslPolicy) + if err != nil { + t.Errorf("ParseResourceURL(%+v) = %v, want nil", tps.SslPolicy, err) + } + + path := resourceID.ResourcePath() + want := "global/sslPolicies/test-policy" + + if path != want { + t.Errorf("tps ssl policy = %q, want %q", path, want) + } +} + +func TestGetSslPolicyLink(t *testing.T) { + t.Parallel() + j := newTestJig(t) + + testCases := []struct { + desc string + fc *frontendconfigv1beta1.FrontendConfig + want *string + }{ + { + desc: "Empty frontendconfig", + fc: nil, + want: nil, + }, + { + desc: "frontendconfig with no ssl policy", + fc: &frontendconfigv1beta1.FrontendConfig{Spec: frontendconfigv1beta1.FrontendConfigSpec{}}, + want: nil, + }, + { + desc: "frontendconfig with ssl policy", + fc: &frontendconfigv1beta1.FrontendConfig{Spec: frontendconfigv1beta1.FrontendConfigSpec{SslPolicy: utils.NewStringPointer("test-policy")}}, + want: utils.NewStringPointer("global/sslPolicies/test-policy"), + }, + { + desc: "frontendconfig with empty string ssl policy", + fc: &frontendconfigv1beta1.FrontendConfig{Spec: frontendconfigv1beta1.FrontendConfigSpec{SslPolicy: utils.NewStringPointer("")}}, + want: utils.NewStringPointer(""), + }, + } + + for _, tc := range testCases { + l7 := L7{runtimeInfo: &L7RuntimeInfo{FrontendConfig: tc.fc}, cloud: j.fakeGCE, scope: meta.Global} + result, err := l7.getSslPolicyLink() + if err != nil { + t.Errorf("desc: %q, l7.getSslPolicyLink() = %v, want nil", tc.desc, err) + } + + if !reflect.DeepEqual(result, tc.want) { + t.Errorf("desc: %q, l7.getSslPolicyLink() = %v, want %+v", tc.desc, result, tc.want) + } + } +} + +func TestEnsureSslPolicy(t *testing.T) { + t.Parallel() + j := newTestJig(t) + + testCases := []struct { + desc string + fc *frontendconfigv1beta1.FrontendConfig + proxy *composite.TargetHttpsProxy + want string + }{ + { + desc: "Empty frontendconfig", + proxy: &composite.TargetHttpsProxy{Name: "test-proxy-0"}, + fc: nil, + want: "", + }, + { + desc: "frontendconfig with no ssl policy", + fc: &frontendconfigv1beta1.FrontendConfig{Spec: frontendconfigv1beta1.FrontendConfigSpec{}}, + proxy: &composite.TargetHttpsProxy{Name: "test-proxy-1"}, + want: "", + }, + { + desc: "frontendconfig with ssl policy", + fc: &frontendconfigv1beta1.FrontendConfig{Spec: frontendconfigv1beta1.FrontendConfigSpec{SslPolicy: utils.NewStringPointer("test-policy")}}, + proxy: &composite.TargetHttpsProxy{Name: "test-proxy-2"}, + want: "global/sslPolicies/test-policy", + }, + { + desc: "proxy with different ssl policy", + fc: &frontendconfigv1beta1.FrontendConfig{Spec: frontendconfigv1beta1.FrontendConfigSpec{SslPolicy: utils.NewStringPointer("test-policy")}}, + proxy: &composite.TargetHttpsProxy{Name: "test-proxy-3", SslPolicy: "global/sslPolicies/wrong-policy"}, + want: "global/sslPolicies/test-policy", + }, + { + desc: "proxy with ssl policy and frontend config policy is nil", + fc: &frontendconfigv1beta1.FrontendConfig{Spec: frontendconfigv1beta1.FrontendConfigSpec{SslPolicy: nil}}, + proxy: &composite.TargetHttpsProxy{Name: "test-proxy-4", SslPolicy: "global/sslPolicies/test-policy"}, + want: "global/sslPolicies/test-policy", + }, + { + desc: "remove ssl policy", + fc: &frontendconfigv1beta1.FrontendConfig{Spec: frontendconfigv1beta1.FrontendConfigSpec{SslPolicy: utils.NewStringPointer("")}}, + proxy: &composite.TargetHttpsProxy{Name: "test-proxy-5", SslPolicy: "global/sslPolicies/wrong-policy"}, + want: "", + }, + } + + for _, tc := range testCases { + key := meta.GlobalKey(tc.proxy.Name) + if err := composite.CreateTargetHttpsProxy(j.fakeGCE, key, tc.proxy); err != nil { + t.Error(err) + } + l7 := L7{runtimeInfo: &L7RuntimeInfo{FrontendConfig: tc.fc}, cloud: j.fakeGCE, scope: meta.Global} + + if err := l7.ensureSslPolicy(tc.proxy); err != nil { + t.Errorf("desc: %q, l7.ensureSslPolicy() = %v, want nil", tc.desc, err) + } + + result, err := composite.GetTargetHttpsProxy(j.fakeGCE, key, meta.VersionGA) + if err != nil { + t.Error(err) + } + + if result.SslPolicy != tc.want { + t.Errorf("desc: %q, want %q, got %q", tc.desc, tc.want, result.SslPolicy) + } + } +} + // verifyURLMap gets the created URLMap and compares it against an expected one. func verifyURLMap(t *testing.T, j *testJig, feNamer namer_util.IngressFrontendNamer, wantGCEURLMap *utils.GCEURLMap) { t.Helper() diff --git a/pkg/loadbalancers/target_proxies.go b/pkg/loadbalancers/target_proxies.go index 892c52e89e..6daab3ccb0 100644 --- a/pkg/loadbalancers/target_proxies.go +++ b/pkg/loadbalancers/target_proxies.go @@ -19,6 +19,7 @@ package loadbalancers import ( "github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud" "k8s.io/ingress-gce/pkg/composite" + "k8s.io/ingress-gce/pkg/flags" "k8s.io/ingress-gce/pkg/utils" "k8s.io/ingress-gce/pkg/utils/namer" "k8s.io/klog" @@ -133,6 +134,12 @@ func (l *L7) checkHttpsProxy() (err error) { return err } + if flags.F.EnableFrontendConfig { + if err := l.ensureSslPolicy(newProxy); err != nil { + return err + } + } + key, err = l.CreateKey(proxyName) if err != nil { return err @@ -171,8 +178,14 @@ func (l *L7) checkHttpsProxy() (err error) { if err := composite.SetSslCertificateForTargetHttpsProxy(l.cloud, key, proxy, sslCertURLs); err != nil { return err } + } + if flags.F.EnableFrontendConfig { + if err := l.ensureSslPolicy(proxy); err != nil { + return err + } } + l.tps = proxy return nil } @@ -190,3 +203,56 @@ func (l *L7) getSslCertLinkInUse() ([]string, error) { return proxy.SslCertificates, nil } + +// ensureSslPolicy ensures that the SslPolicy described in the frontendconfig is +// properly applied to the proxy. +func (l *L7) ensureSslPolicy(proxy *composite.TargetHttpsProxy) error { + policyLink, err := l.getSslPolicyLink() + if err != nil { + return err + } + + if policyLink != nil && !utils.EqualResourceIDs(*policyLink, proxy.SslPolicy) { + key, err := l.CreateKey(proxy.Name) + if err != nil { + return err + } + if err := composite.SetSslPolicyForTargetHttpsProxy(l.cloud, key, proxy, *policyLink); err != nil { + return err + } + } + return nil +} + +// getSslPolicyLink returns the ref to the ssl policy that is described by the +// frontend config. Since Ssl Policy is a *string, there are three possible I/O situations +// 1) policy is nil -> this returns nil +// 2) policy is an empty string -> this returns an empty string +// 3) policy is non-empty -> this constructs the resourcce path and returns it +func (l *L7) getSslPolicyLink() (*string, error) { + var link string + + if l.runtimeInfo.FrontendConfig == nil { + return nil, nil + } + + policyName := l.runtimeInfo.FrontendConfig.Spec.SslPolicy + if policyName == nil { + return nil, nil + } + if *policyName == "" { + return &link, nil + } + + key, err := l.CreateKey(*policyName) + if err != nil { + return nil, err + } + resourceID := cloud.ResourceID{ + Resource: "sslPolicies", + Key: key, + } + resID := resourceID.ResourcePath() + + return &resID, nil +} diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go index 62f2bf28ac..b49f03c9ce 100644 --- a/pkg/utils/utils.go +++ b/pkg/utils/utils.go @@ -439,3 +439,8 @@ func IsLegacyL4ILBService(svc *api_v1.Service) bool { } return false } + +// NewStringPointer returns a pointer to the provided string literal +func NewStringPointer(s string) *string { + return &s +}