crossplane / crossplane/upjet

`ignore_changes` formatting is wrong for list of lists

Open
#295 0 comments 1 reaction 0 assignees View on GitHub
bug codegen v2
Dominant language
Go
Stars
481
Forks
131
Avg merge
2d 1h
Merged PRs (30d)
11

Description

### What happened?

While working on supporting management policies in no-fork architecture, I noticed that [`getIgnoredFieldsArray()`](https://github.com/crossplane/upjet/blob/main/pkg/resource/ignored.go#L44) returns strings that contain “%s”, when supplied list of lists. It shouldn't do so, because these returned values are not used as format strings in any place. Rather, they are [embedded](https://github.com/crossplane/upjet/blob/main/pkg/terraform/files.go#L85) into `ignore_changes` [lifecycle meta-argument](https://github.com/crossplane/upjet/blob/main/pkg/terraform/files.go#L143).

The bug stems from the confusion around name of the first argument, which is named “format”, but never used as a format string.

As far as I could see, we don't have any [test cases](https://github.com/crossplane/upjet/blob/main/pkg/resource/ignored_test.go) covering list of lists.

It is likely that list of lists occurs rarely, if at all, in any Terraform schema. That's probably why this bug has not been encountered in the wild.

### How can we reproduce it?

I've reproduced the issue, using relevant parts of the code, [in a playground](https://go.dev/play/p/_BTk11wk5jR).

### Resolution

Replacing [`fieldPath+"%s"`](https://github.com/crossplane/upjet/blob/main/pkg/resource/ignored.go#L54) with `fieldPath` should be enough. A test case to go with it would be nice.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.