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

Feat validating admission webhook #207

Merged

Conversation

xuzhu-591
Copy link
Collaborator

Description of your changes

Fixes #

I have:

  • Read and followed Horizon's contribution process.
  • Run make build && make lint to ensure this PR is ready for review.

How has this code been tested

Special notes for your reviewer

}

// Get sends the admission request to the webhook server and returns the response
func (c *HTTPAdmissionClient) Get(ctx context.Context, admitData *Request) (*Response, error) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Method HTTPAdmissionClient.Get has 6 return statements (exceeds 4 allowed).

Interest(*Request) bool
}

func Validating(ctx context.Context, request *Request) error {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function Validating has 7 return statements (exceeds 4 allowed).

Interest(*Request) bool
}

func Validating(ctx context.Context, request *Request) error {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function Validating has a Cognitive Complexity of 22 (exceeds 20 allowed). Consider refactoring.

@xuzhu-591 xuzhu-591 force-pushed the feat-validating-admission-webhook branch 2 times, most recently from be770ae to 6add4c5 Compare December 18, 2023 08:38
@xuzhu-591 xuzhu-591 force-pushed the feat-validating-admission-webhook branch from 6add4c5 to 9944bbb Compare December 25, 2023 09:39
Interest(*Request) bool
}

func Validating(ctx context.Context, request *Request) error {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function Validating has 8 return statements (exceeds 4 allowed).

Interest(*Request) bool
}

func Validating(ctx context.Context, request *Request) error {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function Validating has a Cognitive Complexity of 27 (exceeds 20 allowed). Consider refactoring.

pkg/admission/webhook.go Outdated Show resolved Hide resolved
pkg/admission/webhook.go Outdated Show resolved Hide resolved
Signed-off-by: xu.zhu <[email protected]>
@xuzhu-591 xuzhu-591 force-pushed the feat-validating-admission-webhook branch from 9944bbb to b2ec543 Compare December 28, 2023 11:35
)

// Middleware to validate and mutate admission request
func Middleware(skippers ...middleware.Skipper) gin.HandlerFunc {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function Middleware has a Cognitive Complexity of 21 (exceeds 20 allowed). Consider refactoring.

)

// Middleware to validate and mutate admission request
func Middleware(skippers ...middleware.Skipper) gin.HandlerFunc {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function Middleware has 53 lines of code (exceeds 50 allowed). Consider refactoring.

pkg/admission/webhook.go Outdated Show resolved Hide resolved
pkg/admission/webhook.go Outdated Show resolved Hide resolved
}
}

func Clear() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clear is not called.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

pkg/admission/http.go Outdated Show resolved Hide resolved
pkg/config/admission/config.go Show resolved Hide resolved
pkg/config/admission/config.go Outdated Show resolved Hide resolved
core/middleware/admission/admission.go Show resolved Hide resolved
core/middleware/admission/admission.go Outdated Show resolved Hide resolved
pkg/admission/http.go Outdated Show resolved Hide resolved
Interest(*Request) bool
}

func Validating(ctx context.Context, request *Request) error {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function Validating has 51 lines of code (exceeds 50 allowed). Consider refactoring.

)

// Middleware to validate and mutate admission request
func Middleware(skippers ...middleware.Skipper) gin.HandlerFunc {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function Middleware has 56 lines of code (exceeds 50 allowed). Consider refactoring.

Signed-off-by: xu.zhu <[email protected]>
@xuzhu-591 xuzhu-591 force-pushed the feat-validating-admission-webhook branch from 7226cce to f8e5e61 Compare January 5, 2024 10:15
)

// Middleware to validate and mutate admission request
func Middleware(skippers ...middleware.Skipper) gin.HandlerFunc {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function Middleware has a Cognitive Complexity of 22 (exceeds 20 allowed). Consider refactoring.

Copy link

codeclimate bot commented Jan 5, 2024

Code Climate has analyzed commit f8e5e61 and detected 6 issues on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 6

The test coverage on the diff in this pull request is 74.9% (60% is the threshold).

This pull request will bring the total coverage in the repository to 66.3% (0.0% change).

View more on Code Climate.

@@ -624,7 +631,8 @@ func Init(ctx context.Context, flags *Flags, coreConfig *config.Config) {
middleware.MethodAndPathSkipper(http.MethodPost, regexp.MustCompile("^/apis/core/v[12]/users/login"))),
prehandlemiddle.Middleware(r, manager),
auth.Middleware(rbacAuthorizer, authzSkippers...),
tagmiddle.Middleware(), // tag middleware, parse and attach tagSelector to context
tagmiddle.Middleware(),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comment?

@xuzhu-591 xuzhu-591 merged commit 4c1159d into horizoncd:main Jan 8, 2024
5 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants