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

Validation/Mutating webhook for Fission resources #2577

Closed
wants to merge 20 commits into from
Closed

Conversation

neha-Gupta1
Copy link
Contributor

Description

Added Validation/Mutating Webhooks for Fission resources. It will test all API request before CRUD on resources.

Which issue(s) this PR fixes:

Fixes #

Testing

Checklist:

  • I ran tests as well as code linting locally to verify my changes.
  • I have done manual verification of my changes, changes working as expected.
  • I have added new tests to cover my changes.
  • My changes follow contributing guidelines of Fission.
  • I have signed all of my commits.

pkg/webhook/admission-webhook.go Outdated Show resolved Hide resolved
pkg/webhook/admission-webhook.go Outdated Show resolved Hide resolved
wLogger := logger.Named("webhook")

// Setup a Manager
mgr, err := manager.New(config.GetConfigOrDie(), manager.Options{
Copy link
Member

Choose a reason for hiding this comment

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

Please enable metrics port here. I think we have 8080 across all components.

@@ -17,30 +17,12 @@ package signals

Copy link
Member

Choose a reason for hiding this comment

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

We should remove this package entirely.

// ValidateCreate implements webhook.Validator so a webhook will be registered for the type
func (r *Environment) ValidateCreate() error {
environmentlog.Info("validate create", "name", r.Name)
err := r.Validate()
Copy link
Member

Choose a reason for hiding this comment

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

We can directly return error from Validate call. Rest of code is unnecessary. Please do same changes at rest of places.

return r.Validate()

)

// log is for logging in this package.
var httptriggerlog = logf.Log.WithName("httptrigger-resource")
Copy link
Member

Choose a reason for hiding this comment

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

Please use GetLogger from "utils/loggerfactory" for consistency across Fission source code.


// ValidateCreate implements webhook.Validator so a webhook will be registered for the type
func (r *HTTPTrigger) ValidateCreate() error {
httptriggerlog.Info("validate create", "name", r.Name)
Copy link
Member

Choose a reason for hiding this comment

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

Please change all info statements from debug.

@@ -0,0 +1,84 @@
/*
Copy link
Member

Choose a reason for hiding this comment

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

Please rename file to webhook.go

call k8s API from CLI for all resources
@neha-Gupta1 neha-Gupta1 marked this pull request as ready for review November 3, 2022 05:36
@neha-Gupta1 neha-Gupta1 closed this Nov 3, 2022
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.

None yet

2 participants