From cb99d97a10228d4d4dd8e2eed4ca1a034391d73c Mon Sep 17 00:00:00 2001 From: Max Neverov Date: Sat, 19 Oct 2024 15:55:05 +0200 Subject: [PATCH] Refactoring: make client package for client related annotations. --- .../nginx-configuration/annotations-risk.md | 2 +- internal/ingress/annotations/annotations.go | 6 +-- .../{clientbodybuffersize => client}/main.go | 52 ++++++++++++++----- .../main_test.go | 17 +++--- internal/ingress/controller/controller.go | 5 +- 5 files changed, 56 insertions(+), 26 deletions(-) rename internal/ingress/annotations/{clientbodybuffersize => client}/main.go (59%) rename internal/ingress/annotations/{clientbodybuffersize => client}/main_test.go (85%) diff --git a/docs/user-guide/nginx-configuration/annotations-risk.md b/docs/user-guide/nginx-configuration/annotations-risk.md index 3e3b93986b..0b2cc41ca4 100755 --- a/docs/user-guide/nginx-configuration/annotations-risk.md +++ b/docs/user-guide/nginx-configuration/annotations-risk.md @@ -22,7 +22,7 @@ | CertificateAuth | auth-tls-secret | Medium | location | | CertificateAuth | auth-tls-verify-client | Medium | location | | CertificateAuth | auth-tls-verify-depth | Low | location | -| ClientBodyBufferSize | client-body-buffer-size | Low | location | +| Client | client-body-buffer-size | Low | location | | ConfigurationSnippet | configuration-snippet | Critical | location | | Connection | connection-proxy-header | Low | location | | CorsConfig | cors-allow-credentials | Low | ingress | diff --git a/internal/ingress/annotations/annotations.go b/internal/ingress/annotations/annotations.go index e10cc9be17..42c0d42b81 100644 --- a/internal/ingress/annotations/annotations.go +++ b/internal/ingress/annotations/annotations.go @@ -31,7 +31,7 @@ import ( "k8s.io/ingress-nginx/internal/ingress/annotations/authtls" "k8s.io/ingress-nginx/internal/ingress/annotations/backendprotocol" "k8s.io/ingress-nginx/internal/ingress/annotations/canary" - "k8s.io/ingress-nginx/internal/ingress/annotations/clientbodybuffersize" + "k8s.io/ingress-nginx/internal/ingress/annotations/client" "k8s.io/ingress-nginx/internal/ingress/annotations/connection" "k8s.io/ingress-nginx/internal/ingress/annotations/cors" "k8s.io/ingress-nginx/internal/ingress/annotations/customheaders" @@ -80,7 +80,7 @@ type Ingress struct { BasicDigestAuth auth.Config Canary canary.Config CertificateAuth authtls.Config - ClientBodyBufferSize string + Client client.Config CustomHeaders customheaders.Config ConfigurationSnippet string Connection connection.Config @@ -129,7 +129,7 @@ func NewAnnotationFactory(cfg resolver.Resolver) map[string]parser.IngressAnnota "BasicDigestAuth": auth.NewParser(auth.AuthDirectory, cfg), "Canary": canary.NewParser(cfg), "CertificateAuth": authtls.NewParser(cfg), - "ClientBodyBufferSize": clientbodybuffersize.NewParser(cfg), + "Client": client.NewParser(cfg), "CustomHeaders": customheaders.NewParser(cfg), "ConfigurationSnippet": snippet.NewParser(cfg), "Connection": connection.NewParser(cfg), diff --git a/internal/ingress/annotations/clientbodybuffersize/main.go b/internal/ingress/annotations/client/main.go similarity index 59% rename from internal/ingress/annotations/clientbodybuffersize/main.go rename to internal/ingress/annotations/client/main.go index c0fa797139..b1a04c2210 100644 --- a/internal/ingress/annotations/clientbodybuffersize/main.go +++ b/internal/ingress/annotations/client/main.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package clientbodybuffersize +package client import ( networking "k8s.io/api/networking/v1" @@ -27,7 +27,7 @@ const ( clientBodyBufferSizeAnnotation = "client-body-buffer-size" ) -var clientBodyBufferSizeConfig = parser.Annotation{ +var clientAnnotations = parser.Annotation{ Group: "backend", Annotations: parser.AnnotationFields{ clientBodyBufferSizeAnnotation: { @@ -42,30 +42,54 @@ var clientBodyBufferSizeConfig = parser.Annotation{ }, } -type clientBodyBufferSize struct { +type Config struct { + BodyBufferSize string `json:"bodyBufferSize"` +} + +// Equal tests for equality between two Configuration types +func (l1 *Config) Equal(l2 *Config) bool { + if l1 == l2 { + return true + } + if l1 == nil || l2 == nil { + return false + } + if l1.BodyBufferSize != l2.BodyBufferSize { + return false + } + + return true +} + +type client struct { r resolver.Resolver annotationConfig parser.Annotation } -// NewParser creates a new clientBodyBufferSize annotation parser +// NewParser creates a new client annotation parser func NewParser(r resolver.Resolver) parser.IngressAnnotation { - return clientBodyBufferSize{ + return client{ r: r, - annotationConfig: clientBodyBufferSizeConfig, + annotationConfig: clientAnnotations, } } -func (cbbs clientBodyBufferSize) GetDocumentation() parser.AnnotationFields { - return cbbs.annotationConfig.Annotations +func (c client) GetDocumentation() parser.AnnotationFields { + return c.annotationConfig.Annotations } // Parse parses the annotations contained in the ingress rule -// used to add an client-body-buffer-size to the provided locations -func (cbbs clientBodyBufferSize) Parse(ing *networking.Ingress) (interface{}, error) { - return parser.GetStringAnnotation(clientBodyBufferSizeAnnotation, ing, cbbs.annotationConfig.Annotations) +// used to add an client related configuration to the provided locations. +func (c client) Parse(ing *networking.Ingress) (interface{}, error) { + config := &Config{} + + var err error + config.BodyBufferSize, err = parser.GetStringAnnotation(clientBodyBufferSizeAnnotation, ing, c.annotationConfig.Annotations) + + return config, err } -func (cbbs clientBodyBufferSize) Validate(anns map[string]string) error { - maxrisk := parser.StringRiskToRisk(cbbs.r.GetSecurityConfiguration().AnnotationsRiskLevel) - return parser.CheckAnnotationRisk(anns, maxrisk, clientBodyBufferSizeConfig.Annotations) +func (c client) Validate(annotations map[string]string) error { + maxRisk := parser.StringRiskToRisk(c.r.GetSecurityConfiguration().AnnotationsRiskLevel) + return parser.CheckAnnotationRisk(annotations, maxRisk, clientAnnotations.Annotations) } diff --git a/internal/ingress/annotations/clientbodybuffersize/main_test.go b/internal/ingress/annotations/client/main_test.go similarity index 85% rename from internal/ingress/annotations/clientbodybuffersize/main_test.go rename to internal/ingress/annotations/client/main_test.go index 62257aeb91..fb62ff1b10 100644 --- a/internal/ingress/annotations/clientbodybuffersize/main_test.go +++ b/internal/ingress/annotations/client/main_test.go @@ -14,14 +14,15 @@ See the License for the specific language governing permissions and limitations under the License. */ -package clientbodybuffersize +package client import ( "testing" api "k8s.io/api/core/v1" networking "k8s.io/api/networking/v1" - meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/ingress-nginx/internal/ingress/annotations/parser" "k8s.io/ingress-nginx/internal/ingress/resolver" ) @@ -48,7 +49,7 @@ func TestParse(t *testing.T) { } ing := &networking.Ingress{ - ObjectMeta: meta_v1.ObjectMeta{ + ObjectMeta: metav1.ObjectMeta{ Name: "foo", Namespace: api.NamespaceDefault, }, @@ -58,9 +59,13 @@ func TestParse(t *testing.T) { for _, testCase := range testCases { ing.SetAnnotations(testCase.annotations) //nolint:errcheck // Ignore the error since invalid cases will be checked with expected results - result, _ := ap.Parse(ing) - if result != testCase.expected { - t.Errorf("expected %v but returned %v, annotations: %s", testCase.expected, result, testCase.annotations) + res, _ := ap.Parse(ing) + c, ok := res.(*Config) + if !ok { + t.Fatal("expected a client.Config type") + } + if c.BodyBufferSize != testCase.expected { + t.Errorf("expected %v but returned %v, annotations: %s", testCase.expected, res, testCase.annotations) } } } diff --git a/internal/ingress/controller/controller.go b/internal/ingress/controller/controller.go index aa8f4c4b92..ea7a4aadbb 100644 --- a/internal/ingress/controller/controller.go +++ b/internal/ingress/controller/controller.go @@ -32,6 +32,8 @@ import ( "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/wait" clientset "k8s.io/client-go/kubernetes" + "k8s.io/klog/v2" + "k8s.io/ingress-nginx/internal/ingress/annotations" "k8s.io/ingress-nginx/internal/ingress/annotations/canary" "k8s.io/ingress-nginx/internal/ingress/annotations/log" @@ -47,7 +49,6 @@ import ( "k8s.io/ingress-nginx/internal/nginx" "k8s.io/ingress-nginx/pkg/apis/ingress" utilingress "k8s.io/ingress-nginx/pkg/util/ingress" - "k8s.io/klog/v2" ) const ( @@ -1502,7 +1503,7 @@ func (n *NGINXController) createServers(data []*ingress.Ingress, func locationApplyAnnotations(loc *ingress.Location, anns *annotations.Ingress) { loc.BasicDigestAuth = anns.BasicDigestAuth - loc.ClientBodyBufferSize = anns.ClientBodyBufferSize + loc.ClientBodyBufferSize = anns.Client.BodyBufferSize loc.CustomHeaders = anns.CustomHeaders loc.ConfigurationSnippet = anns.ConfigurationSnippet loc.CorsConfig = anns.CorsConfig