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.

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

Implement support for Internal/Regional Static IP for L7-ILB #1174

Merged
merged 2 commits into from
Jul 29, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
46 changes: 41 additions & 5 deletions pkg/annotations/ingress.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,11 @@ limitations under the License.
package annotations

import (
"errors"
"strconv"

"k8s.io/api/networking/v1beta1"
"k8s.io/ingress-gce/pkg/flags"
)

const (
Expand All @@ -34,12 +36,19 @@ const (
// rules for port 443 based on the TLS section.
AllowHTTPKey = "kubernetes.io/ingress.allow-http"

// StaticIPNameKey tells the Ingress controller to use a specific GCE
// GlobalStaticIPNameKey tells the Ingress controller to use a specific GCE
// static ip for its forwarding rules. If specified, the Ingress controller
// assigns the static ip by this name to the forwarding rules of the given
// Ingress. The controller *does not* manage this ip, it is the users
// responsibility to create/delete it.
StaticIPNameKey = "kubernetes.io/ingress.global-static-ip-name"
GlobalStaticIPNameKey = "kubernetes.io/ingress.global-static-ip-name"

// RegionalStaticIPNameKey tells the Ingress controller to use a specific GCE
// internal static ip for its forwarding rules. If specified, the Ingress controller
// assigns the static ip by this name to the forwarding rules of the given
// Ingress. The controller *does not* manage this ip, it is the users
// responsibility to create/delete it.
RegionalStaticIPNameKey = "kubernetes.io/ingress.regional-static-ip-name"

// PreSharedCertKey represents the specific pre-shared SSL
// certificate for the Ingress controller to use. The controller *does not*
Expand All @@ -50,7 +59,7 @@ const (

// IngressClassKey picks a specific "class" for the Ingress. The controller
// only processes Ingresses with this annotation either unset, or set
// to either gceIngessClass or the empty string.
// to either gceIngressClass or the empty string.
IngressClassKey = "kubernetes.io/ingress.class"
GceIngressClass = "gce"
GceMultiIngressClass = "gce-multi-cluster"
Expand Down Expand Up @@ -131,8 +140,35 @@ func (ing *Ingress) UseNamedTLS() string {
return val
}

func (ing *Ingress) StaticIPName() string {
val, ok := ing.v[StaticIPNameKey]
func (ing *Ingress) StaticIPName() (string, error) {
if !flags.F.EnableL7Ilb {
return ing.GlobalStaticIPName(), nil
}

globalIp := ing.GlobalStaticIPName()
regionalIp := ing.RegionalStaticIPName()

if globalIp != "" && regionalIp != "" {
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess you need to validate if this is a L7 ILB then just return regionalStaticIP and ignore GlobalStaticIP?

vice versa for L7 XLB?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True, but I wanted to have a check for if the user specified both by accident

return "", errors.New("Error: both global-static-ip and regional-static-ip cannot be specified")
}

if regionalIp != "" {
return regionalIp, nil
}

return globalIp, nil
}

func (ing *Ingress) GlobalStaticIPName() string {
val, ok := ing.v[GlobalStaticIPNameKey]
if !ok {
return ""
}
return val
}

func (ing *Ingress) RegionalStaticIPName() string {
val, ok := ing.v[RegionalStaticIPNameKey]
if !ok {
return ""
}
Expand Down
42 changes: 36 additions & 6 deletions pkg/annotations/ingress_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,28 +21,49 @@ import (

"k8s.io/api/networking/v1beta1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/ingress-gce/pkg/flags"
)

func TestIngress(t *testing.T) {
for _, tc := range []struct {
desc string
ing *v1beta1.Ingress
allowHTTP bool
useNamedTLS string
staticIPName string
ingressClass string
wantErr bool
}{
{
desc: "Empty ingress",
ing: &v1beta1.Ingress{},
allowHTTP: true, // defaults to true.
},
{
desc: "Global and Regional StaticIP Specified",
ing: &v1beta1.Ingress{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
AllowHTTPKey: "false",
IngressClassKey: "gce",
PreSharedCertKey: "shared-cert-key",
StaticIPNameKey: "1.2.3.4",
GlobalStaticIPNameKey: "1.2.3.4",
RegionalStaticIPNameKey: "10.0.0.0",
IngressClassKey: GceL7ILBIngressClass,
},
},
},
ingressClass: GceL7ILBIngressClass,
staticIPName: "",
allowHTTP: true,
wantErr: true,
},
{
desc: "Test most annotations",
ing: &v1beta1.Ingress{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
AllowHTTPKey: "false",
IngressClassKey: "gce",
PreSharedCertKey: "shared-cert-key",
GlobalStaticIPNameKey: "1.2.3.4",
},
},
},
Expand All @@ -53,14 +74,23 @@ func TestIngress(t *testing.T) {
},
} {
ing := FromIngress(tc.ing)

if tc.ingressClass == GceL7ILBIngressClass {
flags.F.EnableL7Ilb = true
}

if x := ing.AllowHTTP(); x != tc.allowHTTP {
t.Errorf("ingress %+v; AllowHTTP() = %v, want %v", tc.ing, x, tc.allowHTTP)
}
if x := ing.UseNamedTLS(); x != tc.useNamedTLS {
t.Errorf("ingress %+v; UseNamedTLS() = %v, want %v", tc.ing, x, tc.useNamedTLS)
}
if x := ing.StaticIPName(); x != tc.staticIPName {
t.Errorf("ingress %+v; StaticIPName() = %v, want %v", tc.ing, x, tc.staticIPName)
staticIp, err := ing.StaticIPName()
if (err != nil) != tc.wantErr {
t.Errorf("ingress: %+v, err = %v, wantErr = %v", tc.ing, err, tc.wantErr)
}
if staticIp != tc.staticIPName {
t.Errorf("ingress %+v; GlobalStaticIPName() = %v, want %v", tc.ing, staticIp, tc.staticIPName)
}
if x := ing.IngressClass(); x != tc.ingressClass {
t.Errorf("ingress %+v; IngressClass() = %v, want %v", tc.ing, x, tc.ingressClass)
Expand Down
Loading