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

Validate the label/annotation in validate method of Kanister function #3032

Merged
merged 11 commits into from
Aug 20, 2024

Conversation

viveksinghggits
Copy link
Contributor

@viveksinghggits viveksinghggits commented Aug 14, 2024

Change Overview

As part of Validate method of a Kanister function we were just checking if the correct arguments to that functions are provided or not. But As part of this PR #3013 we are introducing two more two more args (podLabels and podAnnotations) to the kanister functions that create new pod.
This PR enhances the Validate method of the functions so that we can check validity of the configured labels and annotations as well.
This is the Validate method also eventually gets called (by blueprint's validating webhook) when we create a blueprint, so implementing this make sure that if we create a blueprint by specifying wrong labels/annotations to the kanister functions, the validating webhook rejects the request.

Pull request type

Please check the type of change your PR introduces:

  • 🚧 Work in Progress
  • 🌈 Refactoring (no functional changes, no api changes)
  • 🐹 Trivial/Minor
  • 🐛 Bugfix
  • 🌻 Feature
  • 🗺️ Documentation
  • 🤖 Test
  • 🏗️ Build

Issues

  • fixes #issue-number

Test Plan

  • 💪 Manual
  • ⚡ Unit test
  • 💚 E2E
~/opensource/kanister/pkg/blueprint/validate (validate-ann-labels-functions*) » go test -check.f "TestValidateAnnLabelArgs"
Passed the 'validation of phase backup in action backup' check.. ✅
Passed the 'validation of phase backup in action backup' check.. ✅
Passed the 'validation of phase backup in action backup' check.. ✅
Failed the 'validation of phase backup in action backup' check.. ❌
Failed the 'validation of phase backup in action backup' check.. ❌
Failed the 'validation of phase backup in action backup' check.. ❌
Failed the 'validation of phase backup in action backup' check.. ❌
Passed the 'validation of phase backup in action backup' check.. ✅
OK: 1 passed
PASS
ok      github.com/kanisterio/kanister/pkg/blueprint/validate   0.915s

Since these args are going to be named the same for all the kanister functions
that support these arguments, it's better to have them as a const and use that
everywhere.
It will help us when we try to get labels and annotations from blueprints.
1. Remove duplicate const declarations
2. Reformat function calls to have them in newline
Since these args are going to be named the same for all the kanister functions
that support these arguments, it's better to have them as a const and use that
everywhere.
It will help us when we try to get labels and annotations from blueprints.
@viveksinghggits viveksinghggits force-pushed the validate-ann-labels-functions branch from bd3d154 to a7e8e71 Compare August 19, 2024 10:40
While validating the labels and annotations, when the validate resulted
into error, we returned `nil` instead of `err`.
This commit fixes that and adds some comments into some functions.
@viveksinghggits viveksinghggits changed the title Validate the label/annotation validity in validate method of kan func… Validate the label/annotation in validate method of Kanister function Aug 19, 2024
pkg/function/utils.go Show resolved Hide resolved
pkg/function/utils.go Outdated Show resolved Hide resolved
pkg/blueprint/validate/validate_test.go Show resolved Hide resolved
Base automatically changed from ann-labels-functions to master August 20, 2024 12:37
Copy link
Contributor

@PrasadG193 PrasadG193 left a comment

Choose a reason for hiding this comment

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

LGTM

@mergify mergify bot merged commit 314b65d into master Aug 20, 2024
18 checks passed
@mergify mergify bot deleted the validate-ann-labels-functions branch August 20, 2024 15:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants