implement terraform support #8

nabuskey · 2023-08-09T23:37:40Z

This became a large PR because I had to fix issues caused by merge conflict in the previous PR. Changes to CRDs introduced in this PR are supposed to be there in the main branch to begin with.

The meat of the PR is TF stuff.

Supports two workflows. For example variables given by this:

variable "name" {
  description = "Name of the VPC and EKS Cluster"
  type        = string
  default     = "emr-eks-ack"
}

# This lacks the default value, therefore it is a required field. 
variable "region" {
  description = "Region"
  type        = string
}

Generation of fields only. Meaning it allows you to create a file per module for relevant field values only. Output for the above input would be:

properties:
  name:
    type: string
    description: Name of the VPC and EKS Cluster
    default: emr-eks-ack
  region:
    type: string
    description: Region
required:
  - region

Generate then insert these fields to an object specified by json path in a template. For example, if the specified jsonpath is .spec.parameters[0] and the given template looks like this.

apiVersion: scaffolder.backstage.io/v1beta3
kind: Template
spec:
  parameters:
    - title: Choose Resource
      description: Select a AWS resource to add to your repository.
      properties:
        path:
          type: string
          description: path to place this file into
          default: kustomize/base
        name:
          type: string
          description: na

The output is

apiVersion: scaffolder.backstage.io/v1beta3
kind: Template
spec:
  parameters:
    - title: Choose Resource
      description: Select a AWS resource to add to your repository.
      properties:
        path:
          type: string
          description: path to place this file into
          default: kustomize/base
        name:
          type: string
          description: Name of the VPC and EKS Cluster # Overwritten by variables value. 
          default: emr-eks-ack
        region:
          type: string
          description: Region
      required: 
        - region

Current supported behaviour is merge keys and if duplicate keys are present, the ones in variables take precedent.

pkg/cmd/terraform.go

nimakaviani · 2023-08-10T22:59:47Z

pkg/cmd/terraform.go

+func init() {
+	tfCmd.Flags().Uint32Var(&depth, "depth", 2, "depth from given directory to search for TF modules")
+	tfCmd.Flags().StringVarP(&templatePath, "templatePath", "t", "scaffolding/template.yaml", "path to the template to be augmented with backstage info")
+	tfCmd.Flags().StringVarP(&insertionPoint, "insertAt", "p", "", "jq path within the template to insert backstage info")


probably requires more hints. also should it come with a default of placing it under spec.parameters[0] if empty?

I also wonder why we didnt take the same approach of adding a resource.enum list under the properties element and create separate yaml files for the terraform module added

also why do we need a json path? can these params go anywhere other than under properties?

I considered it having a default value of spec.parameters[0] but I wanted default behaviour to just generate properties and requirements.

This approach allows you to have one template per module. I noticed having many different yaml files with oneOf property slows down form rendering in Backstage and defaults are not handled correctly.

json path is needed to insert these to wherever users want to insert it. It's possible to have nested objects in json schema form. So if they have a template and want to insert TF properties at spec.params[0].terraformInput, they can do so.

pkg/cmd/terraform.go

nimakaviani · 2023-08-10T23:51:04Z

pkg/cmd/terraform.go

+	return strings.ReplaceAll(input, " ", "")
+}
+
+func isPrimitive(s string) bool {
+	return s == "string" || s == "number" || s == "bool"
+}
+
+func isNestedPrimitive(s string) bool {
+	nested := strings.HasPrefix(s, "object(") || strings.HasPrefix(s, "map(") || strings.HasPrefix(s, "list(")
+	if nested {
+		return isPrimitive(getNestedType(s))
+	}
+	return false
+}
+
+func getNestedType(s string) string {
+	if strings.HasPrefix(s, "object(") {
+		return strings.TrimSuffix(strings.SplitAfterN(s, "object(", 1)[1], ")")
+	}
+	if strings.HasPrefix(s, "map(") {
+		return strings.TrimSuffix(strings.SplitAfterN(s, "map(", 2)[1], ")")
+	}
+	if strings.HasPrefix(s, "list(") {
+		return strings.TrimSuffix(strings.SplitAfterN(s, "list(", 2)[1], ")")
+	}
+	return s
+}


why are these functions not included under utils.go? also if the functions in utils.go are only used by the terraform command, why are we splitting them out?

These are meant for TF only for now. I think it's possible to re-use these for openapi but I really did not want to make changes to CRD cmd for this PR.

The functions in utils.go are meant to be used by others including crd cmd. I just didn't want to touch it for this.

nimakaviani · 2023-08-10T23:55:45Z

pkg/cmd/fakes/terraform/valid/output/full-template.yaml

+          description: VPC CIDR
+          default: 10.1.0.0/16
+      required:
+        - awsResources


nimakaviani · 2023-08-10T23:58:17Z

pkg/cmd/terraform.go

+
+func init() {
+	tfCmd.Flags().Uint32Var(&depth, "depth", 2, "depth from given directory to search for TF modules")
+	tfCmd.Flags().StringVarP(&templatePath, "templatePath", "t", "scaffolding/template.yaml", "path to the template to be augmented with backstage info")


since we have this both for the CRD and TF, shall we push it under templateCmd?

I also think this is obscure unintuitive info that if template info is not provided, you will still get some parameters only output. shall we make it explicit by creating a separate flag that is xor to templatePath?

Yes that's what I wanted as a final product. But not for this PR.

nimakaviani · 2023-08-10T23:59:03Z

pkg/cmd/terraform_test.go

+	. "github.com/onsi/gomega"
+)
+
+var _ = Describe("Terraform Template", func() {


we test all the happy paths. can we test for some error verifications etc too?

Yes we should.

pkg/cmd/terraform.go

nimakaviani · 2023-08-11T00:11:32Z

pkg/cmd/crd.go

@@ -4,14 +4,13 @@ import (
 	"errors"
 	"fmt"
 	"io"
-	"io/ioutil"


ugh, sucks that all these got overwritten.

nimakaviani · 2023-08-11T00:30:13Z

pkg/cmd/terraform.go

+func getModules(inputDir string, currentDepth uint32) []string {
+	if currentDepth > depth {
+		return nil
+	}
+	if tfconfig.IsModuleDir(inputDir) {
+		return []string{inputDir}
+	}
+	base, _ := filepath.Abs(inputDir)
+	files, _ := os.ReadDir(base)
+	out := make([]string, 1)
+	for _, file := range files {
+		f := filepath.Join(base, file.Name())
+		mods := getModules(f, currentDepth+1)
+		out = append(out, mods...)


can we combine this and func defs from the crd command with some neat closure and make them one function? look very similar structurally.

I'd rather not, this is intended for TF only and I don't want to have TF specific handling stuff in a common util file.

i thought the closure that we path to directory traversal would be command specific. so for tf and crd you handle things separately in their respective code. but the directory traversal bit can be generalized

I refactored it and handle errors now.

What we look for is different in each case. In TF we look for directories, while we look for files in CRDs. So not much to have in the util file.

nabuskey · 2023-08-16T00:47:23Z

Refactored CRD cmd to use util functions. CRD cmd now allows for the same functionalities as terraform one.

Generation of property files only. These are yaml files with properties key under which schema information is located. See: pkg/cmd/fakes/crd/valid/output/properties-awsblueprints.io.xcdn.yaml
Original behaviour where all resources are pushed under a template. : pkg/cmd/fakes/crd/valid/output/full-template-oneof.yaml
Generation of a template file per definition. pkg/cmd/fakes/crd/valid/output/full-template-awsblueprints.io.xcdn.yaml and pkg/cmd/fakes/crd/valid/output/full-template-sparkoperator.k8s.io.sparkapplication.yaml

- add the raw flag for when only OpenAPI spec is required - remove implicit assumption on the use of flags - simplify the code

implement terraform support

0389cde

nabuskey requested a review from nimakaviani August 9, 2023 23:37

nabuskey added 2 commits August 9, 2023 16:41

close file

108b5f7

use yaml matcher

025ff08

nimakaviani reviewed Aug 11, 2023

View reviewed changes

consolidate cmd flags

e5d17e9

nabuskey linked an issue Aug 11, 2023 that may be closed by this pull request

Make the depth configurable for the CRD command #10

Closed

nabuskey added 5 commits August 11, 2023 15:45

document tf cmd behaviour

10c7322

refactor file finders

202bec4

remove unnecessary lines

37bb5bb

add more tests

3e9d372

refactor crd command to use util functions where possible

2e99eb7

nimakaviani added 6 commits August 27, 2023 16:52

fix crd template compilation

8d933b8

fix terraform templating

a5b1552

- add the raw flag for when only OpenAPI spec is required - remove implicit assumption on the use of flags - simplify the code

combine common parts of the code across terraform and crds

d5ab2b9

more cleanup

7d41717

further cleanup to the code

e6651c8

combine more of similar code into one place

c6dabc5

nimakaviani merged commit bc321d9 into main Sep 5, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

implement terraform support #8

implement terraform support #8

nabuskey commented Aug 9, 2023

nimakaviani Aug 10, 2023

nimakaviani Aug 11, 2023

nabuskey Aug 11, 2023

nimakaviani Aug 10, 2023

nabuskey Aug 11, 2023 •

edited

Loading

nimakaviani Aug 10, 2023

nimakaviani Aug 10, 2023

nimakaviani Aug 11, 2023

nabuskey Aug 11, 2023

nimakaviani Aug 10, 2023

nabuskey Aug 11, 2023

nimakaviani Aug 11, 2023

nimakaviani Aug 11, 2023

nabuskey Aug 11, 2023

nimakaviani Aug 11, 2023

nabuskey Aug 12, 2023

nabuskey commented Aug 16, 2023

implement terraform support #8

implement terraform support #8

Conversation

nabuskey commented Aug 9, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nabuskey Aug 11, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nabuskey commented Aug 16, 2023

nabuskey Aug 11, 2023 •

edited

Loading