Skip to content

Commit

Permalink
Merge pull request #344 from ulucinar/canonical-json
Browse files Browse the repository at this point in the history
Add json.Canonicalize to compute a canonical form of serialized JSON objects
  • Loading branch information
ulucinar authored Feb 14, 2024
2 parents f6a1a0d + 3372192 commit 780c97f
Show file tree
Hide file tree
Showing 8 changed files with 170 additions and 2 deletions.
44 changes: 44 additions & 0 deletions pkg/config/canonical.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
// SPDX-FileCopyrightText: 2024 The Crossplane Authors <https://crossplane.io>
//
// SPDX-License-Identifier: Apache-2.0

package config

import (
"github.com/pkg/errors"

"github.com/crossplane/upjet/pkg/resource/json"
)

const (
errFmtNotJSONString = "parameter at path %q with value %v is not a (JSON) string"
errFmtCanonicalize = "failed to canonicalize the parameter at path %q"
)

// CanonicalizeJSONParameters returns a ConfigurationInjector that computes
// and stores the canonical forms of the JSON documents for the specified list
// of top-level Terraform configuration arguments. Please note that currently
// only top-level configuration arguments are supported by this function.
func CanonicalizeJSONParameters(tfPath ...string) ConfigurationInjector {
return func(jsonMap map[string]any, tfMap map[string]any) error {
for _, param := range tfPath {
p, ok := tfMap[param]
if !ok {
continue
}
s, ok := p.(string)
if !ok {
return errors.Errorf(errFmtNotJSONString, param, p)
}
if s == "" {
continue
}
cJSON, err := json.Canonicalize(s)
if err != nil {
return errors.Wrapf(err, errFmtCanonicalize, param)
}
tfMap[param] = cJSON
}
return nil
}
}
2 changes: 1 addition & 1 deletion pkg/config/resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -512,7 +512,7 @@ type CustomDiff func(diff *terraform.InstanceDiff, state *terraform.InstanceStat
// values from the specified managed resource into the specified configuration
// map. jsonMap is the map obtained by converting the `spec.forProvider` using
// the JSON tags and tfMap is obtained by using the TF tags.
type ConfigurationInjector func(jsonMap map[string]any, tfMap map[string]any)
type ConfigurationInjector func(jsonMap map[string]any, tfMap map[string]any) error

// SchemaElementOptions represents schema element options for the
// schema elements of a Resource.
Expand Down
4 changes: 3 additions & 1 deletion pkg/controller/external_tfpluginsdk.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,9 @@ func getExtendedParameters(ctx context.Context, tr resource.Terraformed, externa
if err != nil {
return nil, errors.Wrap(err, "cannot get JSON map for the managed resource's spec.forProvider value")
}
config.TerraformConfigurationInjector(m, params)
if err := config.TerraformConfigurationInjector(m, params); err != nil {
return nil, errors.Wrap(err, "cannot invoke the configured TerraformConfigurationInjector")
}
}

tfID, err := config.ExternalName.GetIDFn(ctx, externalName, params, ts.Map())
Expand Down
33 changes: 33 additions & 0 deletions pkg/resource/json/canonical.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// SPDX-FileCopyrightText: 2024 The Crossplane Authors <https://crossplane.io>
//
// SPDX-License-Identifier: Apache-2.0

package json

import (
jsoniter "github.com/json-iterator/go"
"github.com/pkg/errors"
)

const (
errFmtJSONUnmarshal = "failed to unmarshal the JSON document: %s"
errFmtJSONMarshal = "failed to marshal the dictionary: %v"
)

var (
cJSON = jsoniter.Config{
SortMapKeys: true,
}.Froze()
)

// Canonicalize minifies and orders the keys of the specified JSON document
// to return a canonical form of it, along with any errors encountered during
// the process.
func Canonicalize(json string) (string, error) {
var m map[string]any
if err := cJSON.Unmarshal([]byte(json), &m); err != nil {
return "", errors.Wrapf(err, errFmtJSONUnmarshal, json)
}
buff, err := cJSON.Marshal(m)
return string(buff), errors.Wrapf(err, errFmtJSONMarshal, m)
}
67 changes: 67 additions & 0 deletions pkg/resource/json/canonical_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
// SPDX-FileCopyrightText: 2024 The Crossplane Authors <https://crossplane.io>
//
// SPDX-License-Identifier: Apache-2.0

package json

import (
"os"
"path/filepath"
"strings"
"testing"

"github.com/crossplane/crossplane-runtime/pkg/test"
"github.com/google/go-cmp/cmp"
"github.com/pkg/errors"
)

func TestCanonicalize(t *testing.T) {
tests := map[string]struct {
inputFile string
expectedFile string
err error
}{
"SuccessfulConversion": {
inputFile: "policy.json",
expectedFile: "policy_canonical.json",
},
"NoopConversion": {
inputFile: "policy_canonical.json",
expectedFile: "policy_canonical.json",
},
"InvalidJSON": {
inputFile: "invalid.json",
err: errors.Wrap(errors.New(`ReadString: expects " or n, but found }, error found in #10 byte of ...|"a": "b",}|..., bigger context ...|{"a": "b",}|...`), `failed to unmarshal the JSON document: {"a": "b",}`),
},
}

for name, tc := range tests {
t.Run(name, func(t *testing.T) {
input, err := os.ReadFile(filepath.Join("testdata", tc.inputFile))
if err != nil {
t.Fatalf("Failed to read the input file: %v", err)
}

expectedOutput := ""
if tc.expectedFile != "" {
output, err := os.ReadFile(filepath.Join("testdata", tc.expectedFile))
if err != nil {
t.Fatalf("Failed to read expected the output file: %v", err)
}
expectedOutput = strings.Join(strings.Split(strings.TrimSpace(string(output)), "\n")[3:], "\n")
}

inputJSON := strings.Join(strings.Split(strings.TrimSpace(string(input)), "\n")[3:], "\n")
canonicalJSON, err := Canonicalize(inputJSON)
if err != nil {
if diff := cmp.Diff(tc.err, err, test.EquateErrors()); diff != "" {
t.Fatalf("Canonicalize(...): -wantErr, +gotErr: %s", diff)
}
return
}
if diff := cmp.Diff(expectedOutput, canonicalJSON); diff != "" {
t.Errorf("Canonicalize(...): -want, +got: \n%s", diff)
}
})
}
}
4 changes: 4 additions & 0 deletions pkg/resource/json/testdata/invalid.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// SPDX-FileCopyrightText: 2024 The Crossplane Authors <https://crossplane.io>
//
// SPDX-License-Identifier: Apache-2.0
{"a": "b",}
14 changes: 14 additions & 0 deletions pkg/resource/json/testdata/policy.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// SPDX-FileCopyrightText: 2024 The Crossplane Authors <https://crossplane.io>
//
// SPDX-License-Identifier: Apache-2.0
{
"Rules": [
{
"ResourceType": "collection",
"Resource": [
"collection/example-collection-2"
]
}
],
"AWSOwnedKey": true
}
4 changes: 4 additions & 0 deletions pkg/resource/json/testdata/policy_canonical.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// SPDX-FileCopyrightText: 2024 The Crossplane Authors <https://crossplane.io>
//
// SPDX-License-Identifier: Apache-2.0
{"AWSOwnedKey":true,"Rules":[{"Resource":["collection/example-collection-2"],"ResourceType":"collection"}]}

0 comments on commit 780c97f

Please sign in to comment.