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

Added changes for resource transformation feature #215

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lalat-das
Copy link
Collaborator

What this PR does / why we need it: This adds necessary data-structure and api.proto changes for resource transformation feature

Which issue(s) this PR fixes (optional)
Closes # pb-4427

Special notes for your reviewer:

make proto succeeds
image

Signed-off-by: Lalatendu Das <ldas@purestorage.com>
@github-actions
Copy link

OSS Scan Results:

Title Severity Package Name CVEs Fix version Introduced
Improper Input Validation high sigs.k8s.io/aws-iam-authenticator/pkg/token ['CVE-2022-2385'] ['0.5.9'] ['github.com/portworx/px-backup-api@0.0.0', 'sigs.k8s.io/aws-iam-authenticator/pkg/token@0.5.5']

Total issues: 1

@github-actions
Copy link

License Evaluation Results:

Title Package Name Package Version Severity License Info Introduced Dependency Type

Total License Issues: 0

message ResourceTransformTemplate {
// Resource is GroupVersionKind for k8s resources
// should be in format `group/version/kind"
//ResourceType string `json:"resource"`
Copy link
Contributor

Choose a reason for hiding this comment

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

remove the above line. Do we need json tag mentioned in the comment?

@@ -876,6 +932,120 @@ message SchedulePolicyOwnershipUpdateRequest {

message SchedulePolicyOwnershipUpdateResponse {}

service ResourceTransform {
// Creates new schedule policy.
Copy link
Contributor

Choose a reason for hiding this comment

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

Please correct comments in all the API.
schedule policy --> ResourceTransform

map<string, string> labels = 2;
}
// Define ResourceTransformEnumerateResponse struct

Copy link
Contributor

Choose a reason for hiding this comment

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

typo: Remove extra empty line between comment and def

ResourceTransformOpsType resource_transform_ops_type = 3;
// Operation to be performed on path
// add/modify/delete/replace/jsonPatch
ResourceTransformValueType resource_transfor_value_type = 4;
Copy link
Contributor

Choose a reason for hiding this comment

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

typo: resource_transfor_value_type -> resource_transform_value_type

string Path = 1;
// Value for given k8s path
string value = 2;
// Type of value specified int/bool/string/slice/keypair
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like the comment of resource_transform_ops_type and resource_transfor_value_type are interchanged

// stores all the stand alone resource transformation rules which are not part any predefined resource transformation template yet
repeated ResourceTransformTemplate resource_transform_templates = 24;
// stores array of predefined resource transformation template fed during restore
repeated ObjectRef resource_transformation_ref = 25;
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we planning to support more one resourceTransform object reference in a restore object? Why it has to be an array?

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.

2 participants