Skip to content

Commit

Permalink
Refactoring: make client package for client related annotations.
Browse files Browse the repository at this point in the history
  • Loading branch information
mneverov committed Oct 20, 2024
1 parent 0edf16f commit cb99d97
Show file tree
Hide file tree
Showing 5 changed files with 56 additions and 26 deletions.
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

0 comments on commit cb99d97

Please sign in to comment.