Skip to content

Commit

Permalink
Fix for Annotation was not propagated to the route when changes made …
Browse files Browse the repository at this point in the history 
…to exisiting ingress object

BZ https://bugzilla.redhat.com/show_bug.cgi?id=1860136
  • Loading branch information
@miheer
miheer committed Nov 24, 2020
1 parent 77e8248 commit f0010b4
Show file tree
Hide file tree
Showing 2 changed files with 96 additions and 15 deletions.
21 changes: 15 additions & 6 deletions pkg/route/ingress/ingress.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -425,7 +425,6 @@ func (c *Controller) sync(key queueKey) error {
}

var errs []error

// add the new routes
for _, route := range creates {
if err := createRouteWithName(c.routeClient, ingress, route, c.expectations); err != nil {
Expand All @@ -439,13 +438,18 @@ func (c *Controller) sync(key queueKey) error {
if err != nil {
return err
}
data = []byte(fmt.Sprintf(`[{"op":"replace","path":"/spec","value":%s}]`, data))
annotations, err := json.Marshal(&route.Annotations)
if err != nil {
return err
}
data = []byte(fmt.Sprintf(`[{"op":"replace","path":"/spec","value":%s},`+
`{"op":"replace","path":"/metadata/annotations","value":%s}]`,
data, annotations))
_, err = c.routeClient.Routes(route.Namespace).Patch(context.TODO(), route.Name, types.JSONPatchType, data, metav1.PatchOptions{})
if err != nil {
errs = append(errs, err)
}
}

// purge any previously managed routes
for _, route := range old {
if err := c.routeClient.Routes(route.Namespace).Delete(context.TODO(), route.Name, metav1.DeleteOptions{}); err != nil && !kerrors.IsNotFound(err) {
Expand Down Expand Up @@ -582,15 +586,20 @@ func routeMatchesIngress(
secretLister corelisters.SecretLister,
serviceLister corelisters.ServiceLister,
) bool {
var match_annotations bool
match := route.Spec.Host == rule.Host &&
route.Spec.Path == path.Path &&
route.Spec.To.Name == path.Backend.ServiceName &&
route.Spec.WildcardPolicy == routev1.WildcardPolicyNone &&
len(route.Spec.AlternateBackends) == 0
if !match {
return false
if route.Annotations != nil && ingress.Annotations != nil {
match_annotations = reflect.DeepEqual(route.Annotations, ingress.Annotations)
if !match && !match_annotations {
return false
} else if !match {
return false
}
}

targetPort, err := targetPortForService(ingress.Namespace, path, serviceLister)
if err != nil {
// not valid
Expand Down
90 changes: 81 additions & 9 deletions pkg/route/ingress/ingress_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -870,7 +870,7 @@ func TestController_sync(t *testing.T) {
wantRoutePatches: []clientgotesting.PatchActionImpl{
{
Name: "1-abcdef",
Patch: []byte(`[{"op":"replace","path":"/spec","value":{"host":"test.com","path":"/","to":{"kind":"","name":"service-1","weight":null},"port":{"targetPort":"http"}}}]`),
Patch: []byte(`[{"op":"replace","path":"/spec","value":{"host":"test.com","path":"/","to":{"kind":"","name":"service-1","weight":null},"port":{"targetPort":"http"}}},{"op":"replace","path":"/metadata/annotations","value":null}]`),
},
},
},
Expand Down Expand Up @@ -1134,7 +1134,7 @@ func TestController_sync(t *testing.T) {
wantRoutePatches: []clientgotesting.PatchActionImpl{
{
Name: "1-abcdef",
Patch: []byte(`[{"op":"replace","path":"/spec","value":{"host":"test.com","path":"/","to":{"kind":"","name":"service-1","weight":null},"port":{"targetPort":"http"}}}]`),
Patch: []byte(`[{"op":"replace","path":"/spec","value":{"host":"test.com","path":"/","to":{"kind":"","name":"service-1","weight":null},"port":{"targetPort":"http"}}},{"op":"replace","path":"/metadata/annotations","value":null}]`),
},
},
},
Expand Down Expand Up @@ -1196,7 +1196,7 @@ func TestController_sync(t *testing.T) {
wantRoutePatches: []clientgotesting.PatchActionImpl{
{
Name: "1-abcdef",
Patch: []byte(`[{"op":"replace","path":"/spec","value":{"host":"test.com","path":"/","to":{"kind":"","name":"service-1","weight":null},"port":{"targetPort":"http"},"tls":{"termination":"edge","certificate":"cert","key":"key","insecureEdgeTerminationPolicy":"Redirect"}}}]`),
Patch: []byte(`[{"op":"replace","path":"/spec","value":{"host":"test.com","path":"/","to":{"kind":"","name":"service-1","weight":null},"port":{"targetPort":"http"},"tls":{"termination":"edge","certificate":"cert","key":"key","insecureEdgeTerminationPolicy":"Redirect"}}},{"op":"replace","path":"/metadata/annotations","value":null}]`),
},
},
},
Expand Down Expand Up @@ -1267,7 +1267,7 @@ func TestController_sync(t *testing.T) {
wantRoutePatches: []clientgotesting.PatchActionImpl{
{
Name: "1-abcdef",
Patch: []byte(`[{"op":"replace","path":"/spec","value":{"host":"test.com","path":"/","to":{"kind":"","name":"service-1","weight":null},"port":{"targetPort":"http"},"tls":{"termination":"passthrough","insecureEdgeTerminationPolicy":"Redirect"}}}]`),
Patch: []byte(`[{"op":"replace","path":"/spec","value":{"host":"test.com","path":"/","to":{"kind":"","name":"service-1","weight":null},"port":{"targetPort":"http"},"tls":{"termination":"passthrough","insecureEdgeTerminationPolicy":"Redirect"}}},` + `{"op":"replace","path":"/metadata/annotations","value":{"route.openshift.io/termination":"passthrough"}}]`),
},
},
},
Expand Down Expand Up @@ -1338,7 +1338,7 @@ func TestController_sync(t *testing.T) {
wantRoutePatches: []clientgotesting.PatchActionImpl{
{
Name: "1-abcdef",
Patch: []byte(`[{"op":"replace","path":"/spec","value":{"host":"test.com","path":"/","to":{"kind":"","name":"service-1","weight":null},"port":{"targetPort":"http"},"tls":{"termination":"reencrypt","certificate":"cert","key":"key","insecureEdgeTerminationPolicy":"Redirect"}}}]`),
Patch: []byte(`[{"op":"replace","path":"/spec","value":{"host":"test.com","path":"/","to":{"kind":"","name":"service-1","weight":null},"port":{"targetPort":"http"},"tls":{"termination":"reencrypt","certificate":"cert","key":"key","insecureEdgeTerminationPolicy":"Redirect"}}},` + `{"op":"replace","path":"/metadata/annotations","value":{"route.openshift.io/termination":"reencrypt"}}]`),
},
},
},
Expand Down Expand Up @@ -1406,7 +1406,7 @@ func TestController_sync(t *testing.T) {
wantRoutePatches: []clientgotesting.PatchActionImpl{
{
Name: "1-abcdef",
Patch: []byte(`[{"op":"replace","path":"/spec","value":{"host":"test.com","path":"/","to":{"kind":"","name":"service-1","weight":null},"port":{"targetPort":"http"},"tls":{"termination":"reencrypt","insecureEdgeTerminationPolicy":"Redirect"}}}]`),
Patch: []byte(`[{"op":"replace","path":"/spec","value":{"host":"test.com","path":"/","to":{"kind":"","name":"service-1","weight":null},"port":{"targetPort":"http"},"tls":{"termination":"reencrypt","insecureEdgeTerminationPolicy":"Redirect"}}},` + `{"op":"replace","path":"/metadata/annotations","value":{"route.openshift.io/termination":"reencrypt"}}]`),
},
},
},
Expand Down Expand Up @@ -1539,7 +1539,7 @@ func TestController_sync(t *testing.T) {
wantRoutePatches: []clientgotesting.PatchActionImpl{
{
Name: "1-abcdef",
Patch: []byte(`[{"op":"replace","path":"/spec","value":{"host":"test.com","path":"/","to":{"kind":"","name":"service-1","weight":null},"port":{"targetPort":"http"}}}]`),
Patch: []byte(`[{"op":"replace","path":"/spec","value":{"host":"test.com","path":"/","to":{"kind":"","name":"service-1","weight":null},"port":{"targetPort":"http"}}},` + `{"op":"replace","path":"/metadata/annotations","value":{"route.openshift.io/termination":"Passthrough"}}]`),
},
},
},
Expand Down Expand Up @@ -1599,7 +1599,7 @@ func TestController_sync(t *testing.T) {
wantRoutePatches: []clientgotesting.PatchActionImpl{
{
Name: "1-abcdef",
Patch: []byte(`[{"op":"replace","path":"/spec","value":{"host":"test.com","path":"/","to":{"kind":"","name":"service-1","weight":null},"port":{"targetPort":"http"},"tls":{"termination":"edge","insecureEdgeTerminationPolicy":"Redirect"}}}]`),
Patch: []byte(`[{"op":"replace","path":"/spec","value":{"host":"test.com","path":"/","to":{"kind":"","name":"service-1","weight":null},"port":{"targetPort":"http"},"tls":{"termination":"edge","insecureEdgeTerminationPolicy":"Redirect"}}},{"op":"replace","path":"/metadata/annotations","value":null}]`),
},
},
},
Expand Down Expand Up @@ -1666,7 +1666,7 @@ func TestController_sync(t *testing.T) {
wantRoutePatches: []clientgotesting.PatchActionImpl{
{
Name: "1-abcdef",
Patch: []byte(`[{"op":"replace","path":"/spec","value":{"host":"test.com","path":"/","to":{"kind":"","name":"service-1","weight":null},"port":{"targetPort":"http"},"tls":{"termination":"edge","certificate":"cert","key":"key2"}}}]`),
Patch: []byte(`[{"op":"replace","path":"/spec","value":{"host":"test.com","path":"/","to":{"kind":"","name":"service-1","weight":null},"port":{"targetPort":"http"},"tls":{"termination":"edge","certificate":"cert","key":"key2"}}},{"op":"replace","path":"/metadata/annotations","value":null}]`),
},
},
},
Expand Down Expand Up @@ -2415,6 +2415,78 @@ func TestController_sync(t *testing.T) {
},
args: queueKey{namespace: "test", name: "1"},
},
{
name: "update route - termination policy changed to passthrough and timeout set",
fields: fields{
i: &ingressLister{Items: []*networkingv1beta1.Ingress{
{
ObjectMeta: metav1.ObjectMeta{
Name: "1",
Namespace: "test",
Annotations: map[string]string{
"route.openshift.io/termination": "passthrough",
"haproxy.router.openshift.io/timeout": "6m",
},
},
Spec: networkingv1beta1.IngressSpec{
TLS: []networkingv1beta1.IngressTLS{
{Hosts: []string{"test.com"}, SecretName: "secret-1"},
},
Rules: []networkingv1beta1.IngressRule{
{
Host: "test.com",
IngressRuleValue: networkingv1beta1.IngressRuleValue{
HTTP: &networkingv1beta1.HTTPIngressRuleValue{
Paths: []networkingv1beta1.HTTPIngressPath{
{
Path: "/", Backend: networkingv1beta1.IngressBackend{
ServiceName: "service-1",
ServicePort: intstr.FromString("http"),
},
},
},
},
},
},
},
},
},
}},
r: &routeLister{Items: []*routev1.Route{
{
ObjectMeta: metav1.ObjectMeta{
Name: "1-abcdef",
Namespace: "test",
OwnerReferences: []metav1.OwnerReference{{APIVersion: "networking.k8s.io/v1beta1", Kind: "Ingress", Name: "1", Controller: &boolTrue}},
},
Spec: routev1.RouteSpec{
Host: "test.com",
Path: "/",
TLS: &routev1.TLSConfig{
Termination: routev1.TLSTerminationEdge,
Certificate: "cert",
Key: "key",
InsecureEdgeTerminationPolicy: routev1.InsecureEdgeTerminationPolicyRedirect,
},
To: routev1.RouteTargetReference{
Name: "service-1",
},
Port: &routev1.RoutePort{
TargetPort: intstr.FromString("http"),
},
WildcardPolicy: routev1.WildcardPolicyNone,
},
},
}},
},
args: queueKey{namespace: "test", name: "1"},
wantRoutePatches: []clientgotesting.PatchActionImpl{
{
Name: "1-abcdef",
Patch: []byte(`[{"op":"replace","path":"/spec","value":{"host":"test.com","path":"/","to":{"kind":"","name":"service-1","weight":null},"port":{"targetPort":"http"},"tls":{"termination":"passthrough","insecureEdgeTerminationPolicy":"Redirect"}}},{"op":"replace","path":"/metadata/annotations","value":{"haproxy.router.openshift.io/timeout":"6m","route.openshift.io/termination":"passthrough"}}]`),
},
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down

0 comments on commit f0010b4

Please sign in to comment.