diff --git a/pkg/annotations/ingress.go b/pkg/annotations/ingress.go index a21c0c5444..808becf17b 100644 --- a/pkg/annotations/ingress.go +++ b/pkg/annotations/ingress.go @@ -89,6 +89,8 @@ const ( // UrlMapKey is the annotation key used by controller to record GCP URL map. UrlMapKey = StatusPrefix + "/url-map" + // UrlMapKey is the annotation key used by controller to record GCP URL map used for Https Redirects only. + RedirectUrlMapKey = StatusPrefix + "/redirect-url-map" // HttpForwardingRuleKey is the annotation key used by controller to record // GCP http forwarding rule. HttpForwardingRuleKey = StatusPrefix + "/forwarding-rule" diff --git a/pkg/apis/frontendconfig/v1beta1/types.go b/pkg/apis/frontendconfig/v1beta1/types.go index bdaafc2fe3..f1763d8b8f 100644 --- a/pkg/apis/frontendconfig/v1beta1/types.go +++ b/pkg/apis/frontendconfig/v1beta1/types.go @@ -35,7 +35,16 @@ type FrontendConfig struct { // FrontendConfigSpec is the spec for a FrontendConfig resource // +k8s:openapi-gen=true type FrontendConfigSpec struct { - SslPolicy *string `json:"sslPolicy,omitempty"` + SslPolicy *string `json:"sslPolicy,omitempty"` + RedirectToHttps *HttpsRedirectConfig `json:"redirectToHttps,omitempty"` +} + +// HttpsRedirectConfig representing the configuration of Https redirects +type HttpsRedirectConfig struct { + Enabled bool `json:"enabled"` + // String representing the HTTP response code + // Options are MOVED_PERMANENTLY_DEFAULT, FOUND, TEMPORARY_REDIRECT, or PERMANENT_REDIRECT + ResponseCodeName string `json:"responseCodeName,omitempty"` } // FrontendConfigStatus is the status for a FrontendConfig resource diff --git a/pkg/apis/frontendconfig/v1beta1/zz_generated.deepcopy.go b/pkg/apis/frontendconfig/v1beta1/zz_generated.deepcopy.go index 5bcb616d45..3a6f115955 100644 --- a/pkg/apis/frontendconfig/v1beta1/zz_generated.deepcopy.go +++ b/pkg/apis/frontendconfig/v1beta1/zz_generated.deepcopy.go @@ -93,6 +93,11 @@ func (in *FrontendConfigSpec) DeepCopyInto(out *FrontendConfigSpec) { *out = new(string) **out = **in } + if in.RedirectToHttps != nil { + in, out := &in.RedirectToHttps, &out.RedirectToHttps + *out = new(HttpsRedirectConfig) + **out = **in + } return } @@ -121,3 +126,19 @@ func (in *FrontendConfigStatus) DeepCopy() *FrontendConfigStatus { in.DeepCopyInto(out) return out } + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *HttpsRedirectConfig) DeepCopyInto(out *HttpsRedirectConfig) { + *out = *in + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new HttpsRedirectConfig. +func (in *HttpsRedirectConfig) DeepCopy() *HttpsRedirectConfig { + if in == nil { + return nil + } + out := new(HttpsRedirectConfig) + in.DeepCopyInto(out) + return out +} diff --git a/pkg/apis/frontendconfig/v1beta1/zz_generated.openapi.go b/pkg/apis/frontendconfig/v1beta1/zz_generated.openapi.go index 27ebcc44f5..700ca98096 100644 --- a/pkg/apis/frontendconfig/v1beta1/zz_generated.openapi.go +++ b/pkg/apis/frontendconfig/v1beta1/zz_generated.openapi.go @@ -90,8 +90,15 @@ func schema_pkg_apis_frontendconfig_v1beta1_FrontendConfigSpec(ref common.Refere Format: "", }, }, + "redirectToHttps": { + SchemaProps: spec.SchemaProps{ + Ref: ref("k8s.io/ingress-gce/pkg/apis/frontendconfig/v1beta1.HttpsRedirectConfig"), + }, + }, }, }, }, + Dependencies: []string{ + "k8s.io/ingress-gce/pkg/apis/frontendconfig/v1beta1.HttpsRedirectConfig"}, } } diff --git a/pkg/loadbalancers/l7.go b/pkg/loadbalancers/l7.go index abbce2d950..1a2d1b753b 100644 --- a/pkg/loadbalancers/l7.go +++ b/pkg/loadbalancers/l7.go @@ -89,6 +89,8 @@ type L7 struct { cloud *gce.Cloud // um is the UrlMap associated with this L7. um *composite.UrlMap + // rum is the Http Redirect only UrlMap associated with this L7. + redirectUm *composite.UrlMap // tp is the TargetHTTPProxy associated with this L7. tp *composite.TargetHttpProxy // tps is the TargetHTTPSProxy associated with this L7. @@ -162,6 +164,13 @@ func (l *L7) edgeHop() error { if err := l.ensureComputeURLMap(); err != nil { return err } + + if flags.F.EnableFrontendConfig { + if err := l.ensureRedirectURLMap(); err != nil { + return fmt.Errorf("ensureRedirectUrlMap() = %v", err) + } + } + if l.runtimeInfo.AllowHTTP { if err := l.edgeHopHttp(); err != nil { return err @@ -383,6 +392,19 @@ func (l *L7) Cleanup(versions *features.ResourceVersions) error { if err := utils.IgnoreHTTPNotFound(composite.DeleteUrlMap(l.cloud, key, versions.UrlMap)); err != nil { return err } + + // Delete RedirectUrlMap if exists + if flags.F.EnableFrontendConfig { + umName := l.namer.RedirectUrlMap() + klog.V(2).Infof("Deleting Redirect URL Map %v", umName) + key, err := l.CreateKey(umName) + if err != nil { + return err + } + if err := utils.IgnoreHTTPNotFound(composite.DeleteUrlMap(l.cloud, key, versions.UrlMap)); err != nil { + return err + } + } return nil } @@ -419,6 +441,16 @@ func (l *L7) getFrontendAnnotations(existing map[string]string) map[string]strin } else { delete(existing, annotations.TargetHttpsProxyKey) } + + // Handle Https Redirect Map + if flags.F.EnableFrontendConfig { + if l.redirectUm != nil { + existing[annotations.RedirectUrlMapKey] = l.redirectUm.Name + } else { + delete(existing, annotations.RedirectUrlMapKey) + } + } + // Note that ingress IP annotation is not deleted when user disables one of http/https. // This is because the promoted static IP is retained for use and will be deleted only // when load-balancer is deleted or user specifies a different IP. diff --git a/pkg/loadbalancers/loadbalancers_test.go b/pkg/loadbalancers/loadbalancers_test.go index 592fa2db37..9ad14f90ef 100644 --- a/pkg/loadbalancers/loadbalancers_test.go +++ b/pkg/loadbalancers/loadbalancers_test.go @@ -1098,6 +1098,54 @@ func TestFrontendConfigSslPolicy(t *testing.T) { } } +func TestFrontendConfigRedirects(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: true, + TLS: []*TLSCerts{createCert("key", "cert", "name")}, + UrlMap: gceUrlMap, + Ingress: newIngress(), + FrontendConfig: &frontendconfigv1beta1.FrontendConfig{Spec: frontendconfigv1beta1.FrontendConfigSpec{RedirectToHttps: &frontendconfigv1beta1.HttpsRedirectConfig{Enabled: true}}}, + } + + l7, err := j.pool.Ensure(lbInfo) + if err != nil { + t.Fatalf("j.pool.Ensure(%v) = %v, want nil", lbInfo, err) + } + + // Only verify HTTPS since the HTTP Target Proxy points to the redirect url map + verifyHTTPSForwardingRuleAndProxyLinks(t, j, l7) + + if l7.redirectUm == nil { + t.Errorf("l7.redirectUm is nil") + } + + tpName := l7.tp.Name + tp, err := composite.GetTargetHttpProxy(j.fakeGCE, meta.GlobalKey(tpName), meta.VersionGA) + if err != nil { + t.Error(err) + } + + resourceID, err := cloud.ParseResourceURL(tp.UrlMap) + if err != nil { + t.Errorf("ParseResourceURL(%+v) = %v, want nil", tp.UrlMap, err) + } + + path := resourceID.ResourcePath() + want := "global/urlMaps/" + l7.redirectUm.Name + + if path != want { + t.Errorf("tps ssl policy = %q, want %q", path, want) + } +} + func TestEnsureSslPolicy(t *testing.T) { t.Parallel() j := newTestJig(t) diff --git a/pkg/loadbalancers/target_proxies.go b/pkg/loadbalancers/target_proxies.go index b9c0f690fa..3e4bbc75b0 100644 --- a/pkg/loadbalancers/target_proxies.go +++ b/pkg/loadbalancers/target_proxies.go @@ -34,14 +34,29 @@ const ( // checkProxy ensures the correct TargetHttpProxy for a loadbalancer func (l *L7) checkProxy() (err error) { - isL7ILB := flags.F.EnableL7Ilb && utils.IsGCEL7ILBIngress(l.runtimeInfo.Ingress) - tr := translator.NewTranslator(isL7ILB, l.namer) + // Get UrlMap Name, could be the url map or the redirect url map + // TODO(shance): move to translator + var umName string + if flags.F.EnableFrontendConfig { + // TODO(shance): check for empty name? + if l.redirectUm != nil && l.runtimeInfo.FrontendConfig.Spec.RedirectToHttps != nil && l.runtimeInfo.FrontendConfig.Spec.RedirectToHttps.Enabled { + umName = l.redirectUm.Name + } else { + umName = l.um.Name + } + } else { + umName = l.um.Name + } - description, err := l.description() + urlMapKey, err := l.CreateKey(umName) if err != nil { return err } - urlMapKey, err := l.CreateKey(l.um.Name) + + isL7ILB := flags.F.EnableL7Ilb && utils.IsGCEL7ILBIngress(l.runtimeInfo.Ingress) + tr := translator.NewTranslator(isL7ILB, l.namer) + + description, err := l.description() if err != nil { return err } diff --git a/pkg/loadbalancers/url_maps.go b/pkg/loadbalancers/url_maps.go index 3c174d064b..9501ae83b9 100644 --- a/pkg/loadbalancers/url_maps.go +++ b/pkg/loadbalancers/url_maps.go @@ -23,6 +23,7 @@ import ( "k8s.io/apimachinery/pkg/util/sets" "k8s.io/ingress-gce/pkg/composite" "k8s.io/ingress-gce/pkg/events" + "k8s.io/ingress-gce/pkg/flags" "k8s.io/ingress-gce/pkg/translator" "k8s.io/ingress-gce/pkg/utils" "k8s.io/klog" @@ -80,6 +81,48 @@ func (l *L7) ensureComputeURLMap() error { return nil } +func (l *L7) ensureRedirectURLMap() error { + feConfig := l.runtimeInfo.FrontendConfig + isL7ILB := flags.F.EnableL7Ilb && utils.IsGCEL7ILBIngress(&l.ingress) + + t := translator.NewTranslator(isL7ILB, l.namer) + env := &translator.Env{FrontendConfig: feConfig, Ing: &l.ingress} + + expectedMap := t.ToRedirectUrlMap(env, l.Versions().UrlMap) + if expectedMap == nil { + return nil + } + + // Cannot enable for internal ingress + if isL7ILB { + return fmt.Errorf("error: cannot enable HTTPS Redirects with L7 ILB") + } + + key, err := l.CreateKey(expectedMap.Name) + if err != nil { + return err + } + + currentMap, err := composite.GetUrlMap(l.cloud, key, expectedMap.Version) + if utils.IgnoreHTTPNotFound(err) != nil { + return err + } + + if currentMap == nil { + if err := composite.CreateUrlMap(l.cloud, key, expectedMap); err != nil { + return err + } + } else if currentMap.DefaultUrlRedirect != expectedMap.DefaultUrlRedirect { + expectedMap.Fingerprint = currentMap.Fingerprint + if err := composite.UpdateUrlMap(l.cloud, key, expectedMap); err != nil { + return err + } + } + + l.redirectUm = expectedMap + return nil +} + // getBackendNames returns the names of backends in this L7 urlmap. func getBackendNames(computeURLMap *composite.UrlMap) ([]string, error) { beNames := sets.NewString() diff --git a/pkg/translator/translator.go b/pkg/translator/translator.go index ca2e71bd25..0d97ddad44 100644 --- a/pkg/translator/translator.go +++ b/pkg/translator/translator.go @@ -194,6 +194,27 @@ func ToCompositeURLMap(g *utils.GCEURLMap, namer namer.IngressFrontendNamer, key return m } +// ToRedirectUrlMap returns the UrlMap used for HTTPS Redirects on a L7 ELB +// This function returns nil if no url map needs to be created +func (t *Translator) ToRedirectUrlMap(env *Env, version meta.Version) *composite.UrlMap { + if env.FrontendConfig == nil || env.FrontendConfig.Spec.RedirectToHttps == nil { + return nil + } + + if !env.FrontendConfig.Spec.RedirectToHttps.Enabled { + return nil + } + + redirectConfig := env.FrontendConfig.Spec.RedirectToHttps + expectedMap := &composite.UrlMap{ + Name: t.FrontendNamer.RedirectUrlMap(), + DefaultUrlRedirect: &composite.HttpRedirectAction{HttpsRedirect: redirectConfig.Enabled, RedirectResponseCode: redirectConfig.ResponseCodeName}, + Version: version, + } + + return expectedMap +} + // getNameForPathMatcher returns a name for a pathMatcher based on the given host rule. // The host rule can be a regex, the path matcher name used to associate the 2 cannot. func getNameForPathMatcher(hostRule string) string { diff --git a/pkg/translator/translator_test.go b/pkg/translator/translator_test.go index 94d48236d2..5c8cdd3ab3 100644 --- a/pkg/translator/translator_test.go +++ b/pkg/translator/translator_test.go @@ -50,6 +50,10 @@ func (n *testNamer) UrlMap() string { return fmt.Sprintf("%s-um", n.prefix) } +func (n *testNamer) RedirectUrlMap() string { + return fmt.Sprintf("%s-rm", n.prefix) +} + func (n *testNamer) SSLCertName(secretHash string) string { return fmt.Sprintf("%s-cert-%s", n.prefix, secretHash) } @@ -111,6 +115,53 @@ func TestToComputeURLMap(t *testing.T) { } } +func TestToRedirectUrlMap(t *testing.T) { + t.Parallel() + + testCases := []struct { + desc string + fc *frontendconfigv1beta1.FrontendConfig + expect *composite.UrlMap + }{ + { + desc: "Not included in FrontendConfig", + expect: nil, + }, + { + desc: "Enabled with no response code set", + fc: &frontendconfigv1beta1.FrontendConfig{Spec: frontendconfigv1beta1.FrontendConfigSpec{RedirectToHttps: &frontendconfigv1beta1.HttpsRedirectConfig{Enabled: true}}}, + expect: &composite.UrlMap{Name: "foo-rm", DefaultUrlRedirect: &composite.HttpRedirectAction{HttpsRedirect: true}, Version: meta.VersionGA}, + }, + { + desc: "Enabled with response code set", + fc: &frontendconfigv1beta1.FrontendConfig{Spec: frontendconfigv1beta1.FrontendConfigSpec{RedirectToHttps: &frontendconfigv1beta1.HttpsRedirectConfig{Enabled: true, ResponseCodeName: "MOVED_PERMANENTLY_DEFAULT"}}}, + expect: &composite.UrlMap{Name: "foo-rm", DefaultUrlRedirect: &composite.HttpRedirectAction{HttpsRedirect: true, RedirectResponseCode: "MOVED_PERMANENTLY_DEFAULT"}, Version: meta.VersionGA}, + }, + { + desc: "Disabled with response code set", + fc: &frontendconfigv1beta1.FrontendConfig{Spec: frontendconfigv1beta1.FrontendConfigSpec{RedirectToHttps: &frontendconfigv1beta1.HttpsRedirectConfig{Enabled: false, ResponseCodeName: "MOVED_PERMANENTLY_DEFAULT"}}}, + expect: nil, + }, + { + desc: "Disabled with with no response code set", + fc: &frontendconfigv1beta1.FrontendConfig{Spec: frontendconfigv1beta1.FrontendConfigSpec{RedirectToHttps: &frontendconfigv1beta1.HttpsRedirectConfig{Enabled: false}}}, + expect: nil, + }, + } + + for _, tc := range testCases { + t.Run(tc.desc, func(t *testing.T) { + tr := NewTranslator(false, &testNamer{"foo"}) + env := &Env{FrontendConfig: tc.fc} + + result := tr.ToRedirectUrlMap(env, meta.VersionGA) + if diff := cmp.Diff(tc.expect, result); diff != "" { + t.Errorf("Unexpected diff from ToRedirectUrlMap() (-want +got):\n%s", diff) + } + }) + } +} + func testCompositeURLMap() *composite.UrlMap { return &composite.UrlMap{ Name: "k8s-um-lb-name", diff --git a/pkg/utils/namer/frontendnamer.go b/pkg/utils/namer/frontendnamer.go index 7890458d10..b3c49ad8b9 100644 --- a/pkg/utils/namer/frontendnamer.go +++ b/pkg/utils/namer/frontendnamer.go @@ -38,6 +38,8 @@ const ( maximumAllowedCombinedLength = 36 // urlMapPrefixV2 is URL map prefix for v2 naming scheme. urlMapPrefixV2 = "um" + // urlMapPrefixV2 is Https-Redirect-Only URL map prefix for v2 naming scheme. + redirectUrlMapPrefixV2 = "rm" // forwardingRulePrefixV2 is http forwarding rule prefix for v2 naming scheme. forwardingRulePrefixV2 = "fr" // httpsForwardingRulePrefixV2 is https forwarding rule prefix for v2 naming scheme. @@ -88,6 +90,11 @@ func (ln *V1IngressFrontendNamer) UrlMap() string { return ln.namer.UrlMap(ln.lbName) } +// UrlMap implements IngressFrontendNamer. +func (ln *V1IngressFrontendNamer) RedirectUrlMap() string { + return ln.namer.RedirectUrlMap(ln.lbName) +} + // SSLCertName implements IngressFrontendNamer. func (ln *V1IngressFrontendNamer) SSLCertName(secretHash string) string { return ln.namer.SSLCertName(ln.lbName, secretHash) @@ -175,6 +182,11 @@ func (vn *V2IngressFrontendNamer) UrlMap() string { return fmt.Sprintf("%s%s-%s-%s", vn.prefix, schemaVersionV2, urlMapPrefixV2, vn.lbName) } +// UrlMap returns the name of URL map. +func (vn *V2IngressFrontendNamer) RedirectUrlMap() string { + return fmt.Sprintf("%s%s-%s-%s", vn.prefix, schemaVersionV2, redirectUrlMapPrefixV2, vn.lbName) +} + // SSLCertName returns the name of the certificate. func (vn *V2IngressFrontendNamer) SSLCertName(secretHash string) string { return fmt.Sprintf("%s%s-%s-%s-%s-%s", vn.prefix, schemaVersionV2, sslCertPrefixV2, vn.clusterUID, vn.lbNameToHash(), secretHash) diff --git a/pkg/utils/namer/interfaces.go b/pkg/utils/namer/interfaces.go index 755d8c302c..6e330c61fc 100644 --- a/pkg/utils/namer/interfaces.go +++ b/pkg/utils/namer/interfaces.go @@ -25,6 +25,8 @@ type IngressFrontendNamer interface { TargetProxy(protocol NamerProtocol) string // UrlMap returns the name of the URL Map. UrlMap() string + // RedirectUrlMap returns the name of the URL Map. + RedirectUrlMap() string // SSLCertName returns the SSL certificate name given secret hash. SSLCertName(secretHash string) string // IsCertNameForLB returns true if certName belongs to this ingress. diff --git a/pkg/utils/namer/namer.go b/pkg/utils/namer/namer.go index f043146d1b..4ad8062ed1 100644 --- a/pkg/utils/namer/namer.go +++ b/pkg/utils/namer/namer.go @@ -43,6 +43,7 @@ const ( forwardingRulePrefix = "fw" httpsForwardingRulePrefix = "fws" urlMapPrefix = "um" + redirectMapPrefix = "rm" // This allows sharing of backends across loadbalancers. backendPrefix = "be" @@ -419,6 +420,11 @@ func (n *Namer) UrlMap(lbName LoadBalancerName) string { return truncate(fmt.Sprintf("%v-%v-%v", n.prefix, urlMapPrefix, lbName)) } +// UrlMap returns the name for the UrlMap for a given load balancer. +func (n *Namer) RedirectUrlMap(lbName LoadBalancerName) string { + return truncate(fmt.Sprintf("%v-%v-%v", n.prefix, redirectMapPrefix, lbName)) +} + // NamedPort returns the name for a named port. func (n *Namer) NamedPort(port int64) string { return fmt.Sprintf("port%v", port)