tokers commented on a change in pull request #779: URL: https://github.com/apache/apisix-ingress-controller/pull/779#discussion_r759791796
########## File path: pkg/kube/translation/ingress.go ########## @@ -87,29 +90,52 @@ func (t *translator) translateIngressV1(ing *networkingv1.Ingress) (*TranslateCo ctx.addUpstream(ups) } uris := []string{pathRule.Path} - if pathRule.PathType != nil && *pathRule.PathType == networkingv1.PathTypePrefix { - // As per the specification of Ingress path matching rule: - // if the last element of the path is a substring of the - // last element in request path, it is not a match, e.g. /foo/bar - // matches /foo/bar/baz, but does not match /foo/barbaz. - // While in APISIX, /foo/bar matches both /foo/bar/baz and - // /foo/barbaz. - // In order to be conformant with Ingress specification, here - // we create two paths here, the first is the path itself - // (exact match), the other is path + "/*" (prefix match). - prefix := pathRule.Path - if strings.HasSuffix(prefix, "/") { - prefix += "*" - } else { - prefix += "/*" + var nginxVars []configv2alpha1.ApisixRouteHTTPMatchExpr + if pathRule.PathType != nil { + if *pathRule.PathType == networkingv1.PathTypePrefix { + // As per the specification of Ingress path matching rule: + // if the last element of the path is a substring of the + // last element in request path, it is not a match, e.g. /foo/bar + // matches /foo/bar/baz, but does not match /foo/barbaz. + // While in APISIX, /foo/bar matches both /foo/bar/baz and + // /foo/barbaz. + // In order to be conformant with Ingress specification, here + // we create two paths here, the first is the path itself + // (exact match), the other is path + "/*" (prefix match). + prefix := pathRule.Path + if strings.HasSuffix(prefix, "/") { + prefix += "*" + } else { + prefix += "/*" + } + uris = append(uris, prefix) + } else if *pathRule.PathType == networkingv1.PathTypeExact { + uris = append(uris, pathRule.Path) + } else if *pathRule.PathType == networkingv1.PathTypeImplementationSpecific && useRegex { + nginxVars = append(nginxVars, configv2alpha1.ApisixRouteHTTPMatchExpr{ + Subject: configv2alpha1.ApisixRouteHTTPMatchExprSubject{ + Scope: configv2alpha1.ScopePath, + }, + Op: configv2alpha1.OpRegexMatch, + Set: []string{}, Review comment: We don't initialize this field. ########## File path: pkg/kube/translation/ingress.go ########## @@ -266,29 +319,52 @@ func (t *translator) translateIngressExtensionsV1beta1(ing *extensionsv1beta1.In ctx.addUpstream(ups) } uris := []string{pathRule.Path} - if pathRule.PathType != nil && *pathRule.PathType == extensionsv1beta1.PathTypePrefix { - // As per the specification of Ingress path matching rule: - // if the last element of the path is a substring of the - // last element in request path, it is not a match, e.g. /foo/bar - // matches /foo/bar/baz, but does not match /foo/barbaz. - // While in APISIX, /foo/bar matches both /foo/bar/baz and - // /foo/barbaz. - // In order to be conformant with Ingress specification, here - // we create two paths here, the first is the path itself - // (exact match), the other is path + "/*" (prefix match). - prefix := pathRule.Path - if strings.HasSuffix(prefix, "/") { - prefix += "*" - } else { - prefix += "/*" + var nginxVars []configv2alpha1.ApisixRouteHTTPMatchExpr + if pathRule.PathType != nil { + if *pathRule.PathType == extensionsv1beta1.PathTypePrefix { + // As per the specification of Ingress path matching rule: + // if the last element of the path is a substring of the + // last element in request path, it is not a match, e.g. /foo/bar + // matches /foo/bar/baz, but does not match /foo/barbaz. + // While in APISIX, /foo/bar matches both /foo/bar/baz and + // /foo/barbaz. + // In order to be conformant with Ingress specification, here + // we create two paths here, the first is the path itself + // (exact match), the other is path + "/*" (prefix match). + prefix := pathRule.Path + if strings.HasSuffix(prefix, "/") { + prefix += "*" + } else { + prefix += "/*" + } + uris = append(uris, prefix) + } else if *pathRule.PathType == extensionsv1beta1.PathTypeExact { + uris = append(uris, pathRule.Path) + } else if *pathRule.PathType == extensionsv1beta1.PathTypeImplementationSpecific && useRegex { + nginxVars = append(nginxVars, configv2alpha1.ApisixRouteHTTPMatchExpr{ + Subject: configv2alpha1.ApisixRouteHTTPMatchExprSubject{ + Scope: configv2alpha1.ScopePath, + }, + Op: configv2alpha1.OpRegexMatch, + Set: []string{}, Review comment: Ditto. ########## File path: pkg/kube/translation/annotations/types.go ########## @@ -18,6 +18,10 @@ import ( "strings" ) +const ( + AnnotationsPrefix = "k8s.apisix.apache.org/" Review comment: Is this constant exported to the outside? If not, we can rename it to `annotationsPrefix`. ########## File path: pkg/kube/translation/ingress.go ########## @@ -175,29 +203,52 @@ func (t *translator) translateIngressV1beta1(ing *networkingv1beta1.Ingress) (*T ctx.addUpstream(ups) } uris := []string{pathRule.Path} - if pathRule.PathType != nil && *pathRule.PathType == networkingv1beta1.PathTypePrefix { - // As per the specification of Ingress path matching rule: - // if the last element of the path is a substring of the - // last element in request path, it is not a match, e.g. /foo/bar - // matches /foo/bar/baz, but does not match /foo/barbaz. - // While in APISIX, /foo/bar matches both /foo/bar/baz and - // /foo/barbaz. - // In order to be conformant with Ingress specification, here - // we create two paths here, the first is the path itself - // (exact match), the other is path + "/*" (prefix match). - prefix := pathRule.Path - if strings.HasSuffix(prefix, "/") { - prefix += "*" - } else { - prefix += "/*" + var nginxVars []configv2alpha1.ApisixRouteHTTPMatchExpr + if pathRule.PathType != nil { + if *pathRule.PathType == networkingv1beta1.PathTypePrefix { + // As per the specification of Ingress path matching rule: + // if the last element of the path is a substring of the + // last element in request path, it is not a match, e.g. /foo/bar + // matches /foo/bar/baz, but does not match /foo/barbaz. + // While in APISIX, /foo/bar matches both /foo/bar/baz and + // /foo/barbaz. + // In order to be conformant with Ingress specification, here + // we create two paths here, the first is the path itself + // (exact match), the other is path + "/*" (prefix match). + prefix := pathRule.Path + if strings.HasSuffix(prefix, "/") { + prefix += "*" + } else { + prefix += "/*" + } + uris = append(uris, prefix) + } else if *pathRule.PathType == networkingv1beta1.PathTypeExact { + uris = append(uris, pathRule.Path) + } else if *pathRule.PathType == networkingv1beta1.PathTypeImplementationSpecific && useRegex { + nginxVars = append(nginxVars, configv2alpha1.ApisixRouteHTTPMatchExpr{ + Subject: configv2alpha1.ApisixRouteHTTPMatchExprSubject{ + Scope: configv2alpha1.ScopePath, + }, + Op: configv2alpha1.OpRegexMatch, + Set: []string{}, Review comment: Ditto. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@apisix.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org