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

Add Webhooks for CR's #274

Merged
merged 6 commits into from
Nov 25, 2022
Merged

Conversation

Madhu-1
Copy link
Member

@Madhu-1 Madhu-1 commented Nov 21, 2022

This adds the pending webhooks for the below CR's

  • ReclaimSpaceJob CR
  • ReclaimSpaceCronJob CR
  • NetworkFence CR
  • csiaddosNode CR

Note:- If any extra fields need to be validated, we can open a separate issue and track it as an enhancement, as this PR adds the basic framework, which will allow us for further enhancements.

Comment on lines +64 to +74
if reflect.DeepEqual(n.Spec.Parameters, oldNetworkFence.Spec.Parameters) {
allErrs = append(allErrs, field.Invalid(field.NewPath("spec").Child("parameters"), n.Spec.Parameters, "parameters cannot be changed"))
}

if n.Spec.Secret.Name != oldNetworkFence.Spec.Secret.Name {
allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "secret", "name"), n.Spec.Secret, "secret name cannot be changed"))
}

if n.Spec.Secret.Namespace != oldNetworkFence.Spec.Secret.Namespace {
allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "secret", "namespace"), n.Spec.Secret, "secret namespace cannot be changed"))
}
Copy link
Member

Choose a reason for hiding this comment

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

just wondering, is there any reason to have these fields as immutable ?

rest of the pr looks good to me.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't see a reason to allow to change the secret once created. Do you see any use case?

Copy link
Member

Choose a reason for hiding this comment

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

I don't see a reason to allow to change the secret once created. Do you see any use case?

no, I can't think of a reason for either side of the argument for secret name/namespace. I was wondering if you had any specific reasons to make them immutable.
But parameters also need to remain unchanged ?

@raghavendra-talur Can you please take a look networkfence CR webhook once?

Copy link
Member Author

Choose a reason for hiding this comment

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

no, I can't think of a reason for either side of the argument for secret name/namespace. I was wondering if you had any specific reasons to make them immutable.
But parameters also need to remain unchanged ?

Am considering this should not be changed once created. if they made any mistake with secrets we should not allow it, the user is free to delete and create it again with new secrets. example secrets and parameters cannot be changed once created in PV and storageclass.

Copy link
Member

@Rakshith-R Rakshith-R left a comment

Choose a reason for hiding this comment

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

Thanks!

the path for the existing CR's like
reclaimspace,networkfence and csinode
object is not correct, this commit
adds the correct path to the project.

Signed-off-by: Madhu Rajanna <madhupr007@gmail.com>
add the missing path for the
ReclaimSpaceCronJob in the PROJECT.

Signed-off-by: Madhu Rajanna <madhupr007@gmail.com>
Add webhook for reclaimspacejobs object to
avoid users from changing the PVC once created.
Below is the command to generate webhook for
reclaimspacejobs object

```bash
$ operator-sdk create webhook --group csiaddons
  --version v1alpha1 --kind ReclaimSpaceJob
  --programmatic-validation
```

Signed-off-by: Madhu Rajanna <madhupr007@gmail.com>
Add webhook for reclaimspacecronjobs object to
avoid users from changing the PVC once created.
Below is the command to generate webhook for
reclaimspacecronjobs object

```bash
$ operator-sdk create webhook --group csiaddons
  --version v1alpha1 --kind ReclaimSpaceCronJob
  --programmatic-validation
```

Signed-off-by: Madhu Rajanna <madhupr007@gmail.com>
Add webhook for networkfence object to
avoid users from changing the driver,secrets
and parameters once created.
Below is the command to generate webhook for
networkfence object

```bash
$ operator-sdk create webhook --group csiaddons
  --version v1alpha1 --kind NetworkFence
  --programmatic-validation
```

Signed-off-by: Madhu Rajanna <madhupr007@gmail.com>
Add webhook for csiaddonsnode object to
avoid users from changing the driver name,
and NodeID once created.
Below is the command to generate webhook for
csiaddonsnode object

```bash
$ operator-sdk create webhook --group csiaddons
  --version v1alpha1 --kind csiAddonsNode
  --programmatic-validation
```

Signed-off-by: Madhu Rajanna <madhupr007@gmail.com>
@Madhu-1 Madhu-1 force-pushed the webhook-reclaimspace branch from 0b41423 to e2a80f8 Compare November 25, 2022 07:09
@mergify mergify bot merged commit 14d52f0 into csi-addons:main Nov 25, 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.

3 participants