-
Notifications
You must be signed in to change notification settings - Fork 17
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
fix(deployments): refactor pull step payload structure #322
Changes from all commits
f2ba823
c698580
22871ac
59da64d
a67e40a
6caa5b3
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 |
---|---|---|
|
@@ -8,6 +8,7 @@ import ( | |
|
||
"github.com/google/uuid" | ||
"github.com/hashicorp/terraform-plugin-framework-jsontypes/jsontypes" | ||
"github.com/hashicorp/terraform-plugin-framework-validators/boolvalidator" | ||
"github.com/hashicorp/terraform-plugin-framework-validators/int64validator" | ||
"github.com/hashicorp/terraform-plugin-framework-validators/stringvalidator" | ||
"github.com/hashicorp/terraform-plugin-framework/attr" | ||
|
@@ -111,6 +112,9 @@ type PullStepModel struct { | |
// Access token for the repository. | ||
AccessToken types.String `tfsdk:"access_token"` | ||
|
||
// IncludeSubmodules is whether to include submodules in the clone. | ||
IncludeSubmodules types.Bool `tfsdk:"include_submodules"` | ||
|
||
// | ||
// Fields for pull_from_{cloud} | ||
// | ||
|
@@ -349,15 +353,16 @@ func (r *DeploymentResource) Schema(_ context.Context, _ resource.SchemaRequest, | |
Default: listdefault.StaticValue(basetypes.NewListValueMust( | ||
types.ObjectType{ | ||
AttrTypes: map[string]attr.Type{ | ||
"type": types.StringType, | ||
"credentials": types.StringType, | ||
"requires": types.StringType, | ||
"directory": types.StringType, | ||
"repository": types.StringType, | ||
"branch": types.StringType, | ||
"access_token": types.StringType, | ||
"bucket": types.StringType, | ||
"folder": types.StringType, | ||
"type": types.StringType, | ||
"credentials": types.StringType, | ||
"requires": types.StringType, | ||
"directory": types.StringType, | ||
"repository": types.StringType, | ||
"branch": types.StringType, | ||
"access_token": types.StringType, | ||
"bucket": types.StringType, | ||
"folder": types.StringType, | ||
"include_submodules": types.BoolType, | ||
}, | ||
}, | ||
[]attr.Value{}, | ||
|
@@ -388,32 +393,39 @@ func (r *DeploymentResource) Schema(_ context.Context, _ resource.SchemaRequest, | |
"directory": schema.StringAttribute{ | ||
Description: "(For type 'set_working_directory') The directory to set as the working directory.", | ||
Optional: true, | ||
Validators: validatorsForConflictingAttributes(nonDirectoryAttributes), | ||
Validators: []validator.String{ | ||
stringvalidator.ConflictsWith(pathExpressionsForAttributes(nonDirectoryAttributes)...), | ||
}, | ||
}, | ||
"repository": schema.StringAttribute{ | ||
Description: "(For type 'git_clone') The URL of the repository to clone.", | ||
Optional: true, | ||
Validators: validatorsForConflictingAttributes(nonGitCloneAttributes), | ||
Validators: stringConflictsWithValidators(nonGitCloneAttributes), | ||
}, | ||
"branch": schema.StringAttribute{ | ||
Description: "(For type 'git_clone') The branch to clone. If not provided, the default branch is used.", | ||
Optional: true, | ||
Validators: validatorsForConflictingAttributes(nonGitCloneAttributes), | ||
Validators: stringConflictsWithValidators(nonGitCloneAttributes), | ||
}, | ||
"access_token": schema.StringAttribute{ | ||
Description: "(For type 'git_clone') Access token for the repository. Refer to a credentials block for security purposes. Used in leiu of 'credentials'.", | ||
Optional: true, | ||
Validators: validatorsForConflictingAttributes(nonGitCloneAttributes), | ||
Validators: stringConflictsWithValidators(nonGitCloneAttributes), | ||
}, | ||
"include_submodules": schema.BoolAttribute{ | ||
Description: "(For type 'git_clone') Whether to include submodules when cloning the repository.", | ||
Optional: true, | ||
Validators: boolConflictsWithValidators(nonGitCloneAttributes), | ||
}, | ||
"bucket": schema.StringAttribute{ | ||
Description: "(For type 'pull_from_*') The name of the bucket where files are stored.", | ||
Optional: true, | ||
Validators: validatorsForConflictingAttributes(nonPullFromAttributes), | ||
Validators: stringConflictsWithValidators(nonPullFromAttributes), | ||
}, | ||
"folder": schema.StringAttribute{ | ||
Description: "(For type 'pull_from_*') The folder in the bucket where files are stored.", | ||
Optional: true, | ||
Validators: validatorsForConflictingAttributes(nonPullFromAttributes), | ||
Validators: stringConflictsWithValidators(nonPullFromAttributes), | ||
}, | ||
}, | ||
}, | ||
|
@@ -430,16 +442,43 @@ func mapPullStepsTerraformToAPI(tfPullSteps []PullStepModel) ([]api.PullStep, di | |
for i := range tfPullSteps { | ||
tfPullStep := tfPullSteps[i] | ||
|
||
apiPullStep := api.PullStep{ | ||
Type: tfPullStep.Type.ValueString(), | ||
pullStepCommon := api.PullStepCommon{ | ||
Credentials: tfPullStep.Credentials.ValueStringPointer(), | ||
Requires: tfPullStep.Requires.ValueStringPointer(), | ||
Directory: tfPullStep.Directory.ValueStringPointer(), | ||
Repository: tfPullStep.Repository.ValueStringPointer(), | ||
Branch: tfPullStep.Branch.ValueStringPointer(), | ||
AccessToken: tfPullStep.AccessToken.ValueStringPointer(), | ||
Bucket: tfPullStep.Bucket.ValueStringPointer(), | ||
Folder: tfPullStep.Folder.ValueStringPointer(), | ||
} | ||
|
||
// Steps that pull from remote storage have the same fields. | ||
// Define the struct here for reuse in each of those cases. | ||
pullStepPullFrom := api.PullStepPullFrom{ | ||
PullStepCommon: pullStepCommon, | ||
Bucket: tfPullStep.Bucket.ValueStringPointer(), | ||
Folder: tfPullStep.Folder.ValueStringPointer(), | ||
} | ||
|
||
var apiPullStep api.PullStep | ||
switch tfPullStep.Type.ValueString() { | ||
case "git_clone": | ||
apiPullStep.PullStepGitClone = &api.PullStepGitClone{ | ||
PullStepCommon: pullStepCommon, | ||
Repository: tfPullStep.Repository.ValueStringPointer(), | ||
Branch: tfPullStep.Branch.ValueStringPointer(), | ||
AccessToken: tfPullStep.AccessToken.ValueStringPointer(), | ||
IncludeSubmodules: tfPullStep.IncludeSubmodules.ValueBoolPointer(), | ||
} | ||
|
||
case "set_working_directory": | ||
apiPullStep.PullStepSetWorkingDirectory = &api.PullStepSetWorkingDirectory{ | ||
Directory: tfPullStep.Directory.ValueStringPointer(), | ||
} | ||
|
||
case "pull_from_azure_blob_storage": | ||
apiPullStep.PullStepPullFromAzureBlobStorage = &pullStepPullFrom | ||
|
||
case "pull_from_gcs": | ||
apiPullStep.PullStepPullFromGCS = &pullStepPullFrom | ||
|
||
case "pull_from_s3": | ||
apiPullStep.PullStepPullFromS3 = &pullStepPullFrom | ||
} | ||
|
||
pullSteps = append(pullSteps, apiPullStep) | ||
|
@@ -456,16 +495,60 @@ func mapPullStepsAPIToTerraform(pullSteps []api.PullStep) ([]PullStepModel, diag | |
for i := range pullSteps { | ||
pullStep := pullSteps[i] | ||
|
||
pullStepModel := PullStepModel{ | ||
Type: types.StringValue(pullStep.Type), | ||
Credentials: types.StringPointerValue(pullStep.Credentials), | ||
Requires: types.StringPointerValue(pullStep.Requires), | ||
Directory: types.StringPointerValue(pullStep.Directory), | ||
Repository: types.StringPointerValue(pullStep.Repository), | ||
Branch: types.StringPointerValue(pullStep.Branch), | ||
AccessToken: types.StringPointerValue(pullStep.AccessToken), | ||
Bucket: types.StringPointerValue(pullStep.Bucket), | ||
Folder: types.StringPointerValue(pullStep.Folder), | ||
var pullStepModel PullStepModel | ||
|
||
// PullStepGitClone | ||
if pullStep.PullStepGitClone != nil { | ||
pullStepModel.Type = types.StringValue("git_clone") | ||
pullStepModel.Repository = types.StringPointerValue(pullStep.PullStepGitClone.Repository) | ||
pullStepModel.Branch = types.StringPointerValue(pullStep.PullStepGitClone.Branch) | ||
pullStepModel.AccessToken = types.StringPointerValue(pullStep.PullStepGitClone.AccessToken) | ||
pullStepModel.IncludeSubmodules = types.BoolPointerValue(pullStep.PullStepGitClone.IncludeSubmodules) | ||
|
||
// common fields | ||
pullStepModel.Credentials = types.StringPointerValue(pullStep.PullStepGitClone.Credentials) | ||
pullStepModel.Requires = types.StringPointerValue(pullStep.PullStepGitClone.Requires) | ||
} | ||
|
||
// PullStepSetWorkingDirectory | ||
if pullStep.PullStepSetWorkingDirectory != nil { | ||
pullStepModel.Type = types.StringValue("set_working_directory") | ||
pullStepModel.Directory = types.StringValue(*pullStep.PullStepSetWorkingDirectory.Directory) | ||
|
||
// common fields not used on this pull step type | ||
} | ||
|
||
// PullStepPullFromAzureBlobStorage | ||
if pullStep.PullStepPullFromAzureBlobStorage != nil { | ||
pullStepModel.Type = types.StringValue("pull_from_azure_blob_storage") | ||
pullStepModel.Bucket = types.StringPointerValue(pullStep.PullStepPullFromAzureBlobStorage.Bucket) | ||
pullStepModel.Folder = types.StringPointerValue(pullStep.PullStepPullFromAzureBlobStorage.Folder) | ||
|
||
// common fields | ||
pullStepModel.Credentials = types.StringPointerValue(pullStep.PullStepPullFromAzureBlobStorage.Credentials) | ||
pullStepModel.Requires = types.StringPointerValue(pullStep.PullStepPullFromAzureBlobStorage.Requires) | ||
} | ||
|
||
// PullStepPullFromGCS | ||
if pullStep.PullStepPullFromGCS != nil { | ||
pullStepModel.Type = types.StringValue("pull_from_gcs") | ||
pullStepModel.Bucket = types.StringPointerValue(pullStep.PullStepPullFromGCS.Bucket) | ||
pullStepModel.Folder = types.StringPointerValue(pullStep.PullStepPullFromGCS.Folder) | ||
|
||
// common fields | ||
pullStepModel.Credentials = types.StringPointerValue(pullStep.PullStepPullFromGCS.Credentials) | ||
pullStepModel.Requires = types.StringPointerValue(pullStep.PullStepPullFromGCS.Requires) | ||
} | ||
|
||
// PullStepPullFromS3 | ||
if pullStep.PullStepPullFromS3 != nil { | ||
pullStepModel.Type = types.StringValue("pull_from_s3") | ||
pullStepModel.Bucket = types.StringPointerValue(pullStep.PullStepPullFromS3.Bucket) | ||
pullStepModel.Folder = types.StringPointerValue(pullStep.PullStepPullFromS3.Folder) | ||
|
||
// common fields | ||
pullStepModel.Credentials = types.StringPointerValue(pullStep.PullStepPullFromS3.Credentials) | ||
pullStepModel.Requires = types.StringPointerValue(pullStep.PullStepPullFromS3.Requires) | ||
} | ||
|
||
tfPullStepsModel = append(tfPullStepsModel, pullStepModel) | ||
|
@@ -911,7 +994,7 @@ func (r *DeploymentResource) ImportState(ctx context.Context, req resource.Impor | |
} | ||
} | ||
|
||
// validatorsForConflictingAttributes provides a list of string validators | ||
// pathExpressionsForAttributes provides a list of path expressions | ||
// used in a ConflictsWith validator for a specific attribute. | ||
// | ||
// This approach is used in lieu of a ConfigValidators method because we take | ||
|
@@ -922,15 +1005,29 @@ func (r *DeploymentResource) ImportState(ctx context.Context, req resource.Impor | |
// be more concise when defining the conflicting attributes. Defining them in | ||
// ConfigValidators instead would be much more verbose, and disconnected from | ||
// the source of truth. | ||
func validatorsForConflictingAttributes(attributes []string) []validator.String { | ||
func pathExpressionsForAttributes(attributes []string) []path.Expression { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Refactored this slightly - it was set up to always return string validators, but now that we have IncludeSubmodules, we need bool validators as well. So I abstracted this one to return only path expressions, then two small helpers below that wrap this function and return the desired validator type. |
||
pathExpressions := make([]path.Expression, 0) | ||
|
||
for _, key := range attributes { | ||
pathExpressions = append(pathExpressions, path.MatchRelative().AtParent().AtName(key)) | ||
} | ||
|
||
return pathExpressions | ||
} | ||
|
||
// stringConflictsWithValidators provides a list of string validators | ||
// for a specific attribute, allowing for more concise schema definitions. | ||
func stringConflictsWithValidators(attributes []string) []validator.String { | ||
return []validator.String{ | ||
stringvalidator.ConflictsWith(pathExpressions...), | ||
stringvalidator.ConflictsWith(pathExpressionsForAttributes(attributes)...), | ||
} | ||
} | ||
|
||
// boolConflictsWithValidators provides a list of bool validators | ||
// for a specific attribute, allowing for more concise schema definitions. | ||
func boolConflictsWithValidators(attributes []string) []validator.Bool { | ||
return []validator.Bool{ | ||
boolvalidator.ConflictsWith(pathExpressionsForAttributes(attributes)...), | ||
} | ||
} | ||
|
||
|
@@ -943,6 +1040,7 @@ var ( | |
"repository", | ||
"branch", | ||
"access_token", | ||
"include_submodules", | ||
} | ||
|
||
pullFromAttributes = []string{ | ||
|
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 is obviously lengthier than the logic we defined before, but because of the payload expected by the API, I have to put configuration under certain fields in the parent struct so that I can give that parent struct the correct JSON struct tag like
prefect.deployments.steps.git_clone
.