Skip to content

Commit

Permalink
feat: sort the variables in the same order from input variables.tf fi…
Browse files Browse the repository at this point in the history
…le. (GoogleCloudPlatform#2591)

Co-authored-by: Zheng Qin <[email protected]>
Co-authored-by: Andrew Peabody <[email protected]>
Co-authored-by: Bharath KKB <[email protected]>
  • Loading branch information
4 people authored Sep 18, 2024
1 parent 0214cb2 commit aa020c1
Show file tree
Hide file tree
Showing 7 changed files with 364 additions and 131 deletions.
256 changes: 128 additions & 128 deletions cli/bpmetadata/int-test/goldens/golden-metadata.yaml

Large diffs are not rendered by default.

52 changes: 49 additions & 3 deletions cli/bpmetadata/tfconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,15 @@ var metaSchema = &hcl.BodySchema{
},
}

var variableSchema = &hcl.BodySchema{
Blocks: []hcl.BlockHeaderSchema{
{
Type: "variable",
LabelNames: []string{"name"},
},
},
}

var metaBlockSchema = &hcl.BodySchema{
Attributes: []hcl.AttributeSchema{
{
Expand Down Expand Up @@ -237,13 +246,21 @@ func getBlueprintInterfaces(configPath string) (*BlueprintInterface, error) {
variables = append(variables, v)
}

// Sort variables
sort.SliceStable(variables, func(i, j int) bool { return variables[i].Name < variables[j].Name })
// Get the varible orders from tf file.
variableOrders, sortErr := getBlueprintVariableOrders(configPath)
if sortErr != nil {
Log.Info("Failed to get variables orders. Fallback to sort by variable names.", sortErr)
sort.SliceStable(variables, func(i, j int) bool { return variables[i].Name < variables[j].Name })
} else {
Log.Info("Sort variables by the original input order.")
sort.SliceStable(variables, func(i, j int) bool {
return variableOrders[variables[i].Name] < variableOrders[variables[j].Name]
})
}

var outputs []*BlueprintOutput
for _, val := range mod.Outputs {
o := getBlueprintOutput(val)

outputs = append(outputs, o)
}

Expand All @@ -256,6 +273,35 @@ func getBlueprintInterfaces(configPath string) (*BlueprintInterface, error) {
}, nil
}

func getBlueprintVariableOrders(configPath string) (map[string]int, error) {
p := hclparse.NewParser()
variableFile, hclDiags := p.ParseHCLFile(filepath.Join(configPath, "variables.tf"))
err := hasHclErrors(hclDiags)
if hclDiags.HasErrors() {
return nil, err
}
variableContent, _, hclDiags := variableFile.Body.PartialContent(variableSchema)
err = hasHclErrors(hclDiags)
if hclDiags.HasErrors() {
return nil, err
}
variableOrderKeys := make(map[string]int)
for i, block := range variableContent.Blocks {
// We only care about variable blocks.
if block.Type != "variable" {
continue
}
// We expect a single label which is the variable name.
if len(block.Labels) != 1 || len(block.Labels[0]) == 0 {
Log.Info("zheng: called here.")
return nil, fmt.Errorf("Vaiable block has no name.")
}

variableOrderKeys[block.Labels[0]] = i
}
return variableOrderKeys, nil
}

// build variable
func getBlueprintVariable(modVar *tfconfig.Variable) *BlueprintVariable {
v := &BlueprintVariable{
Expand Down
45 changes: 45 additions & 0 deletions cli/bpmetadata/tfconfig_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -368,3 +368,48 @@ func TestTFIncompleteProviderVersions(t *testing.T) {
})
}
}

func TestTFVariableSortOrder(t *testing.T) {
tests := []struct {
name string
configPath string
expectOrders map[string]int
expectError bool
}{
{
name: "Variable order should match tf input",
configPath: "sample-module",
expectOrders: map[string]int{
"description": 1,
"project_id": 0,
"regional": 2,
},
expectError: false,
},
{
name: "Empty variable name should create nil order",
configPath: "empty-module",
expectOrders: map[string]int{},
expectError: true,
},
{
name: "No variable name should create nil order",
configPath: "invalid-module",
expectOrders: map[string]int{},
expectError: true,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, err := getBlueprintVariableOrders(path.Join(tfTestdataPath, tt.configPath))
if tt.expectError {
assert.Error(t, err)
assert.Nil(t, got)
} else {
require.NoError(t, err)
assert.Equal(t, got, tt.expectOrders)
}
})
}
}
37 changes: 37 additions & 0 deletions cli/testdata/bpmetadata/tf/empty-module/outputs.tf
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/**
* Copyright 2022 Google LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

// This file was automatically generated from a template in ./autogen/main

output "cluster_id" {
description = "Cluster ID"
}

output "endpoint" {
sensitive = true
description = "Cluster endpoint"
value = local.cluster_endpoint
depends_on = [
/* Nominally, the endpoint is populated as soon as it is known to Terraform.
* However, the cluster may not be in a usable state yet. Therefore any
* resources dependent on the cluster being up will fail to deploy. With
* this explicit dependency, dependent resources can wait for the cluster
* to be up.
*/
google_container_cluster.primary,
google_container_node_pool.pools,
]
}
34 changes: 34 additions & 0 deletions cli/testdata/bpmetadata/tf/empty-module/variables.tf
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/**
* Copyright 2022 Google LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

// This file was automatically generated from a template in ./autogen/main

variable "" { // Empty variable name
description = "The project ID to host the cluster in"
required = false
}

variable "description" {
description = "The description of the cluster"
type = string
default = "some description"
}

variable "regional" {
type = bool
description = "Whether is a regional cluster"
default = true
}
37 changes: 37 additions & 0 deletions cli/testdata/bpmetadata/tf/invalid-module/outputs.tf
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/**
* Copyright 2022 Google LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

// This file was automatically generated from a template in ./autogen/main

output "cluster_id" {
description = "Cluster ID"
}

output "endpoint" {
sensitive = true
description = "Cluster endpoint"
value = local.cluster_endpoint
depends_on = [
/* Nominally, the endpoint is populated as soon as it is known to Terraform.
* However, the cluster may not be in a usable state yet. Therefore any
* resources dependent on the cluster being up will fail to deploy. With
* this explicit dependency, dependent resources can wait for the cluster
* to be up.
*/
google_container_cluster.primary,
google_container_node_pool.pools,
]
}
34 changes: 34 additions & 0 deletions cli/testdata/bpmetadata/tf/invalid-module/variables.tf
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/**
* Copyright 2022 Google LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

// This file was automatically generated from a template in ./autogen/main

variable { // No variable name
description = "The project ID to host the cluster in"
required = false
}

variable "description" {
description = "The description of the cluster"
type = string
default = "some description"
}

variable "regional" {
type = bool
description = "Whether is a regional cluster"
default = true
}

0 comments on commit aa020c1

Please sign in to comment.