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: add terraform provider template #41

Closed
wants to merge 34 commits into from

Conversation

pimielowski
Copy link
Contributor

@pimielowski pimielowski commented Mar 29, 2024

Description

  • Add makefile with common commands
  • Move assets to catalogs related to the specific run
    • Fix assets logic to work only for specific run rather than copy everything twice in generate all scenario
  • Add missing tests
  • Introduce Exclusive parameter
    • Exclusive parameter have a purpose in a spec that need specific template file ie. Provider
  • A lot of minor changes to allow generation both SDK and Terraform Provider

Motivation and Context

#52 #51

How Has This Been Tested?

Run local tests

Checklist

  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes if appropriate.
  • All new and existing tests passed.

Pawel Imielowski and others added 24 commits March 15, 2024 13:12
The RenderTemplate function in the generator has been refactored for clarity and maintainability. Functions to handle each step of template processing (i.e., processing the template and writing data to the file) were extracted. These changes improve the separation of concerns and should make future modifications easier.
# Conflicts:
#	cmd/codegen/config.yaml
#	pkg/properties/normalized.go
Add logic to copy assets only related to the current run
A new Makefile has been added to run build, test SDK, and clean commands. Meanwhile, the go.sum dependencies have been updated, removing some unused dependencies and adding new ones. This should streamline build and test processes, while keeping the dependencies aligned with the project requirements.
Updated the way imports are managed in the "pkg/imports/manager.go" and "pkg/translate/imports.go" files, switching to a v2 manager that handles import types more effectively. Also implemented a new Terraform provider template and enhanced the go.mod file to require additional necessary dependencies. Some other minor code adjustments were made across various files for better alignment with the new implementation.
Various configuration changes have been made to improve the functioning of the Terraform provider. The configuration file `cmd/codegen/config.yaml` has been updated with several new entries and adjustments, and an erroneous import has been removed from `pkg/translate/imports.go`. Several new script files and GitHub workflow files have been added to the `assets/tf_provider/scripts` and `assets/tf_provider/.github/workflows` directories, respectively, to automate tasks like error checking, code formatting, and building. Additionally, several documentation updates have been made, focusing primarily on providing examples and descriptions for the Terraform provider.
The source paths in `config.yaml` were updated to reflect the changed structure of the project. All files from the assets directory and its subdirectories have been moved to the SDK subdirectory to maintain organization and to centralize all necessary files.
Refactored 'Managerv2' to 'ImportManager' in 'pkg/imports', and updated corresponding tests. Added new tests for 'File', 'CopyAssets' and 'Unmarshal'. Also deleted 'tools.go' and modified 'provider.tmpl' to loop over provided specs rather than defining fixed data sources and resources.
Revised the code in generator.go by adding a filter to specifically check and continue processing only if the entry name matches specification name in 'exclusive' mode. This change allows the generator to correctly identify and work with the relevant template files, by ignoring unnecessary or unmatched ones.
… statements

Updated the generator.go package to import additional translation and generation functions. Also, removed all debug print statements from the code.
assets/tf_provider/.github/workflows/build.yml Outdated Show resolved Hide resolved
pkg/commands/codegen/codegen.go Outdated Show resolved Hide resolved
pkg/properties/normalized.go Outdated Show resolved Hide resolved
pkg/translate/imports.go Outdated Show resolved Hide resolved
templates/terraform/resource.tmpl Outdated Show resolved Hide resolved
templates/terraform/test_resource Outdated Show resolved Hide resolved
templates/terraform/test_resource Outdated Show resolved Hide resolved
The RenderImports function has been updated to accept an array of template types. This allows multiple templates to be processed in a single call, simplifying processing whilst retaining existing functionality. This change impacts the addTemplateImports function too, which now loops over each template type input.
@pimielowski pimielowski self-assigned this Apr 10, 2024
@pimielowski pimielowski added the enhancement New feature or request label Apr 10, 2024
@pimielowski pimielowski changed the title Feat add terraform provider template feat: add terraform provider template Apr 10, 2024
@pimielowski pimielowski marked this pull request as ready for review April 10, 2024 09:16
Copy link
Collaborator

@shinmog shinmog left a comment

Choose a reason for hiding this comment

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

There were a couple of files in this PR that reminded me of stuff that existed in the old-style repos for Terraform providers.

assets/tf_provider/.github/workflows/build.yml Outdated Show resolved Hide resolved
assets/tf_provider/.github/workflows/release.yml Outdated Show resolved Hide resolved
---
# generated by https://github.com/hashicorp/terraform-plugin-docs
page_title: "panos_tfid Data Source - panos"
subcategory: ""
Copy link
Collaborator

Choose a reason for hiding this comment

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

This will probably need a subcategory, tho I don't know what it should be at this point...

Just keep it in mind, we'll need to set this subcategory to something that makes sense for users before the provider is released.

Additionally I may need to update this as I'm working on the uuid-style resource, so I'm not sure that the data source or the docs are in a final state right now.

assets/tf_provider/scripts/changelog-links.sh Outdated Show resolved Hide resolved
assets/tf_provider/scripts/errcheck.sh Outdated Show resolved Hide resolved
assets/tf_provider/tf_provider/.goreleaser.yml Outdated Show resolved Hide resolved
Copy link
Collaborator

Choose a reason for hiding this comment

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

So, this isn't really going to be a static file... The version of literally everything in the require block is not set in stone, especially the pango version, which will probably increment in the process of generating the SDK + provider. Some more thought will have to go into how this file is created.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let's leave it now, we don't have to take care of it now. ultimately for MVP we can place latest as a version and remove the sum file.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Same goes for this: this is certainly not a static file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For MVP we can remove it and regenerate through go mod tidy if we have time we will figure it out.

pkg/translate/golang/terraform/provider.go Outdated Show resolved Hide resolved
Comment on lines +3 to +5
terraform_provider_path:
- 'internal'
- 'provider'
Copy link
Collaborator

Choose a reason for hiding this comment

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

So, this is an interesting situation.

If you're making the provider path configurable, then this means that when ./internal/provider/provider.go imports and makes various data sources and resources available to the user to use, then those NewFooBarResource, lines could end up needing to be thelib.NewFooBarResource, instead...

If you're wanting to have that flexibility to be able to have subdirectories in the provider, then it's going to take more work when generating provider.go.

The alternative is that you always default the provider path and don't let it be configurable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was easiest implementation of the where this path should be created -> https://github.com/PaloAltoNetworks/pan-os-codegen/pull/41/files#diff-e48568539bcd76ff7386b82112418449f557df819113f9db8b7e0eb90bc613f2R94
I think I can hardcode the path for the resources like this:

	var tfProviderPath string
	if len(c.Spec.TerraformProviderPath) > 0 {
		tfProviderPath = filepath.Join(c.Spec.TerraformProviderPath...)
	} else {
		tfProviderPath = filepath.Join([]string{"internal", "provider"}...)
	}

And return it in the createFullFilePath function as a part of the filepath.

I understand that if we change this path it could break stuff, maybe in the future we will need that and we need to figure it out how to handle this stuff, but right now I have no time to refactor this to handle custom path, so we can talk about it today how to address it.

@pimielowski
Copy link
Contributor Author

pimielowski commented Apr 10, 2024

There were a couple of files in this PR that reminded me of stuff that existed in the old-style repos for Terraform providers.

@shinmog
I will answer about assets here, those are all your files from v2 branch of the terraform provider -> https://github.com/PaloAltoNetworks/terraform-provider-panos/tree/v2 I was not taking it from terraform-provider-scm, but assuming by your comments, I will take a look and change it

Several scripts and GitHub action workflows related to the Terraform Provider repository have been deleted. Changes have also been made to the generator code to improve file path generation for different types of output files. Release workflow script has been updated to use more recent versions and improve repository content permissions.
@pimielowski pimielowski requested a review from shinmog April 10, 2024 11:21
The tf_provider_scripts section was removed from the cmd/codegen/config.yaml file. This adjustment is required due to a re-structuring of the project, where certain scripts are no longer needed for the Terraform provider.
The code changes remove the 'exclusive' field from spec files and the related logic in the codebase. Instead, custom templates support has been added. This update enables developers to use a custom template for a specific resource rather than being restricted to exclusive resources. The changes also include some refactoring and error handling improvements.
@pimielowski
Copy link
Contributor Author

Ok I add a logic for running code one more time from configs catalog and based on variable custom_template the correct template is selected.


// CreateResourceList creates a string containing a list of resources or data sources based on the elementType parameter.
// The function returns the generated list as a string.
func CreateResourceList(elementType string) string {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This isn't what I have been advocating for / against in yesterday's meeting. Going to take this offline to try and communicate this better.

@migara
Copy link
Member

migara commented Apr 11, 2024

The idea here is not to generate 100% of the pango and panos provider repo content. Please remove the inclusion of all the GitHub actions and anything not related to the provider go code itself @pimielowski

@pimielowski
Copy link
Contributor Author

The idea here is not to generate 100% of the pango and panos provider repo content. Please remove the inclusion of all the GitHub actions and anything not related to the provider go code itself @pimielowski

Removed

@pimielowski
Copy link
Contributor Author

@migara said we don't go with this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants