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

feat: Resource import into state implementation #157

Merged
merged 1 commit into from
Nov 14, 2024
Merged

Conversation

kklimonda-cl
Copy link
Contributor

@kklimonda-cl kklimonda-cl commented Sep 9, 2024

Implement terraform state importing without use of Tfid attribute, by creating provider::panos::create_import_id function that converts given object with location and name attributes into base64 encoded string that can be used as argument for terraform import command and import stanzas.

Closes: #79 #148 #149 #150

@kklimonda-cl kklimonda-cl marked this pull request as ready for review September 23, 2024 08:50
@kklimonda-cl kklimonda-cl changed the title feat: WIP: Remove Tfid from the codebase feat: Resource import into state implementation Sep 23, 2024
@kklimonda-cl kklimonda-cl force-pushed the feat-remove-tfid branch 3 times, most recently from 9a86a23 to 322e09f Compare September 23, 2024 09:13
@kklimonda-cl kklimonda-cl requested a review from jaspalo October 3, 2024 12:37
_ function.Function = &ImportStateCreator{}
)

type ImportStateCreator struct{}
Copy link
Contributor

Choose a reason for hiding this comment

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

question: Would you consider rename the struct for better alignement with the purpose of the function described in the definition section of it?

if resourceFuncs, found := resourceFuncMap[resourceAsn]; !found {
resp.Error = function.ConcatFuncErrors(resp.Error, function.NewArgumentFuncError(0, fmt.Sprintf("Unsupported resource type: %s'", resourceAsn)))
return
} else {
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: the else branch can be skipped since the previous conditional contains return. This may require to have a unique name for the error returned by resourceFuncs.CreateImportId which also eliminates need for error variable declaration.

@@ -57,7 +57,7 @@ func (c *Creator) RenderTemplate() error {
}

// RenderTerraformProviderFile generates a Go file for a Terraform provider based on the provided TerraformProviderFile and Normalization arguments.
func (c *Creator) RenderTerraformProviderFile(spec *properties.Normalization, typ properties.ResourceType) ([]string, []string, error) {
func (c *Creator) RenderTerraformProviderFile(spec *properties.Normalization, typ properties.ResourceType) ([]string, []string, map[string]properties.TerraformProviderSpecMetadata, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: we may benefit from separating the successful return data from the error data here. One of the way is to have a dedicated struct as follows:

type RenderArtifacts struct {
	dataSources  []string
	resources    []string
	partialsMetadata map[string]properties.TerraformProviderSpecMetadata
}

func (c *Creator) RenderTerraformProviderFile(spec *properties.Normalization, typ properties.ResourceType) (*RenderArtifacts, error) {
    ...
}

The change can also improve the function's return statements such as return nil, nil, nil, err in several places by making the manipulation of return data more structured and convenient along the way.


const (
SchemaResource SchemaType = iota
SchemaDataSource SchemaType = iota
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion(non-blocking): This and the following iota can be skipped as follows:

const (
	SchemaResource   SchemaType = iota + 1
	SchemaDataSource SchemaType 
	SchemaCommon     SchemaType
	SchemaProvider   SchemaType
)

The change may also improve the constant group's default zero-value detection since the first constant has non-zero value.

move schemaType type to properties package

Support for importing list resources
@kklimonda-cl kklimonda-cl merged commit 2f37d08 into main Nov 14, 2024
3 checks passed
@kklimonda-cl kklimonda-cl deleted the feat-remove-tfid branch November 14, 2024 13:30
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.

Change (add?) tfid data source into a provider function
2 participants