-
Notifications
You must be signed in to change notification settings - Fork 967
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
ValidatingAdmissionPolicy
resource support
#2576
base: main
Are you sure you want to change the base?
Conversation
Just a heads-up @aayushsss1 since we are currently working on migrating the provider over to the plugin framework we would actually prefer to have this be supported under plugin-framework instead of SDKv2. Let me know if you have any questions, thanks again for picking this up! |
@BBBmau Understood! Is it very different compared to the earlier SDKv2? |
@aayushsss1 You'd want to include the resource in the You can see an example of how this is currently being tested on my fork here: https://github.com/BBBmau/terraform-provider-kubernetes/tree/add-codegen/internal/framework/provider Some plugin-framework resource docs can also be found here: https://developer.hashicorp.com/terraform/plugin/framework/resources#add-resource-to-provider While you work on it feel free to reach out for help! |
@BBBmau I just have a small doubt, the metadata schema uses the earlier sdk v2, do I create a new separate metadata schema based on the plugin framework for the validating_admission_policy resource? |
@aayushsss1 I'm not sure what you're referring to when it comes to the earlier sdk v2, the metadata schema would be implemented the same as how we see it in the linked fork: Let me know if the link answers your question, I could've missed something in regards to |
Understood! I was referring to this earlier - https://github.com/hashicorp/terraform-provider-kubernetes/blob/main/kubernetes/schema_metadata.go |
@BBBmau I've updated the schema in line with the plugin framework. Please have a look at the structure, and let me know if any changes have to be made, I'll get started with the crud and model in the mean time! |
}, | ||
}, | ||
}, | ||
"spec": schema.SingleNestedAttribute{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this looks to be good, I did notice that it's missing the status
block, this can be seen from the api reference: https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.31/#validatingadmissionpolicy-v1-admissionregistration-k8s-io
Did you happen to use the code generation tool for this? Be sure to remove _gen
from the end of all the files if you didn't use the generation tool.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've renamed the files and I've added the status field!
@@ -1,39 +1,39 @@ | |||
module github.com/hashicorp/terraform-provider-kubernetes | |||
|
|||
go 1.21 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what's the reason for the change go.mod and go.sum?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@BBBmau I've reduced the number of changes to the go.mod, the main change is I've added the github.com/hashicorp/terraform-plugin-codegen-kubernetes/
, which in turn has updated the plugin framework and other terraform dependencies.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah the inclusion of github.com/hashicorp/terraform-plugin-codegen-kubernetes/
isn't necessary since it's a tool that you would install separately to help in generating plugin-framework resources.
You can go ahead and revert changes that came from adding it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But isn't github.com/hashicorp/terraform-plugin-codegen-kubernetes/autocrud
required for the CRUD operations?
I had to also utilize it here -
terraform-provider-kubernetes/internal/framework/provider/appsv1/validating_admission_policy.go
Lines 22 to 27 in 93654c5
type ValidatingAdmissionPolicy struct { | |
APIVersion string | |
Kind string | |
clientGetter autocrud.KubernetesClientGetter | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah yeah you're right. Apologies!
It's also worth noting that after reviewing the issue further that that once this PR is complete it unfortunately won't be merged until v3.0.0
due to this resource being supported by default in k8s 1.30
. We intend to do a major version bump in order to support this since we currently only support up to v1.28
. I've added it as part of v3..0.0
milestone as of now.
Of course you can still work on it but wanted to let you know in case you felt that this was on a deadline. We appreciate you working on this!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, thanks for letting me know! If there are any other issues that require immediate attention I'd be glad to help out!
1a3d8a5
to
93654c5
Compare
Description
Acceptance tests
Output from acceptance testing:
Release Note
Release note for CHANGELOG:
References
Closes #2250
Community Note