-
Notifications
You must be signed in to change notification settings - Fork 35
first steps for kms key-ring resource and datasource #897
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
base: main
Are you sure you want to change the base?
Changes from 3 commits
a7259ff
6f6d063
5b59eb6
185fed5
6bcc14a
d320073
bcd0528
99e2b7f
8c654ad
219adac
1186cee
cb23a4e
8d0da16
2732c4a
b483c13
248748e
d41ad9d
77a623e
7df0307
d87d60f
438da14
889b222
0ce560c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
package kms | ||
|
||
import ( | ||
"context" | ||
"github.com/hashicorp/terraform-plugin-framework/datasource" | ||
"github.com/stackitcloud/stackit-sdk-go/services/kms" | ||
) | ||
|
||
var ( | ||
_ datasource.DataSource = &keyRingDataSource{} | ||
) | ||
|
||
func NewKeyRingDataSource() datasource.DataSource { | ||
return &keyRingDataSource{} | ||
} | ||
|
||
type keyRingDataSource struct { | ||
client *kms.APIClient | ||
} | ||
|
||
func (k keyRingDataSource) Metadata(ctx context.Context, request datasource.MetadataRequest, response *datasource.MetadataResponse) { | ||
//TODO implement me | ||
panic("implement me") | ||
} | ||
|
||
func (k keyRingDataSource) Schema(ctx context.Context, request datasource.SchemaRequest, response *datasource.SchemaResponse) { | ||
//TODO implement me | ||
panic("implement me") | ||
} | ||
|
||
func (k keyRingDataSource) Read(ctx context.Context, request datasource.ReadRequest, response *datasource.ReadResponse) { | ||
//TODO implement me | ||
panic("implement me") | ||
} |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,212 @@ | ||||||||||||||||||||||||||||||||||||||
package kms | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
import ( | ||||||||||||||||||||||||||||||||||||||
"context" | ||||||||||||||||||||||||||||||||||||||
"fmt" | ||||||||||||||||||||||||||||||||||||||
"github.com/hashicorp/terraform-plugin-framework-validators/stringvalidator" | ||||||||||||||||||||||||||||||||||||||
"github.com/hashicorp/terraform-plugin-framework/resource" | ||||||||||||||||||||||||||||||||||||||
"github.com/hashicorp/terraform-plugin-framework/resource/schema" | ||||||||||||||||||||||||||||||||||||||
"github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier" | ||||||||||||||||||||||||||||||||||||||
"github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier" | ||||||||||||||||||||||||||||||||||||||
"github.com/hashicorp/terraform-plugin-framework/schema/validator" | ||||||||||||||||||||||||||||||||||||||
"github.com/hashicorp/terraform-plugin-framework/types" | ||||||||||||||||||||||||||||||||||||||
"github.com/hashicorp/terraform-plugin-log/tflog" | ||||||||||||||||||||||||||||||||||||||
"github.com/stackitcloud/stackit-sdk-go/services/kms" | ||||||||||||||||||||||||||||||||||||||
"github.com/stackitcloud/terraform-provider-stackit/stackit/internal/conversion" | ||||||||||||||||||||||||||||||||||||||
"github.com/stackitcloud/terraform-provider-stackit/stackit/internal/core" | ||||||||||||||||||||||||||||||||||||||
kmsUtils "github.com/stackitcloud/terraform-provider-stackit/stackit/internal/services/kms/utils" | ||||||||||||||||||||||||||||||||||||||
"github.com/stackitcloud/terraform-provider-stackit/stackit/internal/utils" | ||||||||||||||||||||||||||||||||||||||
"github.com/stackitcloud/terraform-provider-stackit/stackit/internal/validate" | ||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
type Model struct { | ||||||||||||||||||||||||||||||||||||||
Description types.String `tfsdk:"description"` | ||||||||||||||||||||||||||||||||||||||
DisplayName types.String `tfsdk:"display_name"` | ||||||||||||||||||||||||||||||||||||||
KeyRingId types.String `tfsdk:"key_ring_id"` | ||||||||||||||||||||||||||||||||||||||
Id types.String `tfsdk:"id"` // needed by TF | ||||||||||||||||||||||||||||||||||||||
ProjectId types.String `tfsdk:"project_id"` | ||||||||||||||||||||||||||||||||||||||
RegionId types.String `tfsdk:"region_id"` | ||||||||||||||||||||||||||||||||||||||
|
RegionId types.String `tfsdk:"region_id"` | |
Region types.String `tfsdk:"region"` # small adjustment to stick with the naming conventions across the codebase |
According to https://docs.api.stackit.cloud/documentation/kms/version/v1beta , the KMS API already uses the new multi-region concept.
See e.g. the stackit_routing_table
resource how to implement the multi-region concept. The resource has a region
field which is marked as optional and will use the default_region
configured in the provider as a fallback: https://registry.terraform.io/providers/stackitcloud/stackit/latest/docs/resources/routing_table
- You will need to store the provider configuration within your resource struct:
terraform-provider-stackit/stackit/internal/services/iaasalpha/routingtable/table/resource.go
Line 63 in 8e77675
providerData core.ProviderData terraform-provider-stackit/stackit/internal/services/iaasalpha/routingtable/table/resource.go
Lines 73 to 77 in 8e77675
var ok bool r.providerData, ok = conversion.ParseProviderData(ctx, req.ProviderData, &resp.Diagnostics) if !ok { return } - Field definition:
terraform-provider-stackit/stackit/internal/services/iaasalpha/routingtable/table/resource.go
Lines 161 to 169 in 8e77675
"region": schema.StringAttribute{ Optional: true, // must be computed to allow for storing the override value from the provider Computed: true, Description: "The resource region. If not defined, the provider region is used.", PlanModifiers: []planmodifier.String{ stringplanmodifier.RequiresReplace(), }, }, - Then use this helper func to read the region:
terraform-provider-stackit/stackit/internal/services/iaasalpha/routingtable/table/resource.go
Line 203 in 8e77675
region := r.providerData.GetRegionWithOverride(model.Region)
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.
updated region logic
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.
Don't know if you are aware of this, please don't forget to implement the import.
You can ensure this by adding the code below:
// Ensure the implementation satisfies the expected interfaces.
var (
_ resource.Resource = &keyRingResource{}
_ resource.ResourceWithConfigure = &keyRingResource{}
_ resource.ResourceWithImportState = &keyRingResource{}
)
See the git instance resource how to do it:
terraform-provider-stackit/stackit/internal/services/git/instance/resource.go
Lines 32 to 37 in 8e77675
// Ensure the implementation satisfies the expected interfaces. | |
var ( | |
_ resource.Resource = &gitResource{} | |
_ resource.ResourceWithConfigure = &gitResource{} | |
_ resource.ResourceWithImportState = &gitResource{} | |
) |
Consider doing the same for the datasource, see the git instance datasource for example:
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.
done
Outdated
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.
response.TypeName = request.ProviderTypeName + "kms_key_ring" | |
response.TypeName = request.ProviderTypeName + "_kms_key_ring" |
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.
updated
Outdated
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.
Please consider implementing a unit test for this func.
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.
added unit test
Outdated
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.
Same here
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.
added unit test
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,29 @@ | ||||||
package utils | ||||||
|
||||||
import ( | ||||||
"context" | ||||||
"fmt" | ||||||
"github.com/hashicorp/terraform-plugin-framework/diag" | ||||||
"github.com/stackitcloud/stackit-sdk-go/core/config" | ||||||
"github.com/stackitcloud/stackit-sdk-go/services/kms" | ||||||
"github.com/stackitcloud/terraform-provider-stackit/stackit/internal/core" | ||||||
"github.com/stackitcloud/terraform-provider-stackit/stackit/internal/utils" | ||||||
) | ||||||
|
||||||
func ConfigureClient(ctx context.Context, providerData *core.ProviderData, diags *diag.Diagnostics) *kms.APIClient { | ||||||
apiClientConfigOptions := []config.ConfigurationOption{ | ||||||
config.WithCustomAuth(providerData.RoundTripper), | ||||||
utils.UserAgentConfigOption(providerData.Version), | ||||||
} | ||||||
if providerData.KMSCustomEndpoint != "" { | ||||||
apiClientConfigOptions = append(apiClientConfigOptions, config.WithEndpoint(providerData.KMSCustomEndpoint)) | ||||||
} else { | ||||||
apiClientConfigOptions = append(apiClientConfigOptions, config.WithRegion(providerData.GetRegion())) | ||||||
|
apiClientConfigOptions = append(apiClientConfigOptions, config.WithRegion(providerData.GetRegion())) | |
apiClientConfigOptions = append(apiClientConfigOptions)) |
Since the KMS API already implemented the new multi-region concept, you don't need to set the region here (or better: the SDK should throw an error if you do 😄 ).
Apart from that, consider adding a unit tests for this func, see e.g. https://github.com/stackitcloud/terraform-provider-stackit/blob/8e776757ea2280d1222afe50c3024945b4d99eed/stackit/internal/services/git/utils/util_test.go
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.
updated
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.
Only a nitpick and most of our datasources are also missing this. But this ensures that the Configure method is implemented correct