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

[WIP] Refactoring: make client package for client related annotations. #12212

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
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
2 changes: 1 addition & 1 deletion docs/user-guide/nginx-configuration/annotations-risk.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
Expand Down
6 changes: 3 additions & 3 deletions internal/ingress/annotations/annotations.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -27,7 +27,7 @@ const (
clientBodyBufferSizeAnnotation = "client-body-buffer-size"
)

var clientBodyBufferSizeConfig = parser.Annotation{
var clientAnnotations = parser.Annotation{
Group: "backend",
Annotations: parser.AnnotationFields{
clientBodyBufferSizeAnnotation: {
Expand All @@ -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)
}
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand All @@ -48,7 +49,7 @@ func TestParse(t *testing.T) {
}

ing := &networking.Ingress{
ObjectMeta: meta_v1.ObjectMeta{
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
Namespace: api.NamespaceDefault,
},
Expand All @@ -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)
}
}
}
5 changes: 3 additions & 2 deletions internal/ingress/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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 (
Expand Down Expand Up @@ -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
Expand Down