Skip to content

Commit

Permalink
fix: changelog doesn't show diff for user config enums
Browse files Browse the repository at this point in the history
  • Loading branch information
byashimov committed Dec 16, 2024
1 parent 8b80997 commit 2eee614
Show file tree
Hide file tree
Showing 14 changed files with 142 additions and 73 deletions.
36 changes: 15 additions & 21 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,29 +15,23 @@ nav_order: 1
- Add `aiven_alloydbomni_user` BETA resource and datasource
- Add `aiven_alloydbomni_database` BETA resource and datasource
- Fix `terraform plan`: new resources don't display zero values for user configuration options
- Add `aiven_service_integration` resource field `destination_service_project`: Destination service project name
- Add `aiven_service_integration` resource field `source_service_project`: Source service project name
- Add `aiven_service_integration` datasource field `destination_service_project`: Destination service project name
- Add `aiven_service_integration` datasource field `source_service_project`: Source service project name
- Add `aiven_opensearch` resource field `opensearch_user_config.opensearch_dashboards.multiple_data_source_enabled`:
Enable or disable multiple data sources in OpenSearch Dashboards
- Add `aiven_opensearch` datasource field `opensearch_user_config.opensearch_dashboards.multiple_data_source_enabled`:
Enable or disable multiple data sources in OpenSearch Dashboards
- Change `aiven_account_team_project` resource field `team_type`: remove `organization:billing:read`,
`organization:billing:write`, `organization:network:read`, `organization:network:write`,
`organization:permissions:read`, `organization:permissions:write`, `organization:projects:read`, `organization:projects:write`
- Change `aiven_organization_permission` resource field `permissions.permissions`: remove `organization:billing:read`,
`organization:billing:write`, `organization:network:read`, `organization:network:write`,
- Add `aiven_service_integration` resource and datasource field `destination_service_project`: Destination service project name
- Add `aiven_service_integration` resource and datasource field `source_service_project`: Source service project name
- Change `aiven_account_team_project` resource and datasource field `team_type` (enum): remove
`organization:billing:read`, `organization:billing:write`, `organization:network:read`, `organization:network:write`,
`organization:permissions:read`, `organization:permissions:write`, `organization:projects:read`, `organization:projects:write`
- Change `aiven_pg` resource field `pg_user_config.additional_backup_regions`: remove deprecation
- Change `aiven_project_user` resource field `member_type`: remove `organization:billing:read`,
`organization:billing:write`, `organization:network:read`, `organization:network:write`,
`organization:permissions:read`, `organization:permissions:write`, `organization:projects:read`, `organization:projects:write`
- Change `aiven_account_team_project` datasource field `team_type`: remove `organization:billing:read`,
`organization:billing:write`, `organization:network:read`, `organization:network:write`,
- Add `aiven_alloydbomni` resource and datasource field `alloydbomni_user_config.pg.password_encryption` (enum)
- Change `aiven_flink` resource and datasource field `flink_user_config.flink_version` (enum): add `1.20`
- Add `aiven_opensearch` resource and datasource field
`opensearch_user_config.opensearch_dashboards.multiple_data_source_enabled`: Enable or disable multiple data sources
in OpenSearch Dashboards
- Change `aiven_organization_permission` resource field `permissions.permissions` (enum): remove
`organization:billing:read`, `organization:billing:write`, `organization:network:read`, `organization:network:write`,
`organization:permissions:read`, `organization:permissions:write`, `organization:projects:read`, `organization:projects:write`
- Change `aiven_pg` datasource field `pg_user_config.additional_backup_regions`: remove deprecation
- Change `aiven_project_user` datasource field `member_type`: remove `organization:billing:read`,
- Add `aiven_pg` resource and datasource field `pg_user_config.pg.password_encryption` (enum)
- Change `aiven_pg` resource and datasource field `pg_user_config.additional_backup_regions`: remove deprecation
- Change `aiven_pg` resource and datasource field `pg_user_config.pg_version` (enum): add `17`
- Change `aiven_project_user` resource and datasource field `member_type` (enum): remove `organization:billing:read`,
`organization:billing:write`, `organization:network:read`, `organization:network:write`,
`organization:permissions:read`, `organization:permissions:write`, `organization:projects:read`, `organization:projects:write`

Expand Down
41 changes: 31 additions & 10 deletions changelog/differ.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (
"github.com/samber/lo"
)

func diffItems(resourceType RootType, was, have *Item) (*Diff, error) {
func diffItems(was, have *Item) (*Diff, error) {
// Added or removed
if was == nil || have == nil {
action := AddDiffAction
Expand All @@ -23,8 +23,7 @@ func diffItems(resourceType RootType, was, have *Item) (*Diff, error) {

return &Diff{
Action: action,
RootType: resourceType,
Description: removeEnum(have.Description),
Description: removeEnum(have.Description), // Some fields have enums in the spec description
Item: have,
}, nil
}
Expand Down Expand Up @@ -83,15 +82,14 @@ func diffItems(resourceType RootType, was, have *Item) (*Diff, error) {

return &Diff{
Action: ChangeDiffAction,
RootType: resourceType,
Description: strings.Join(entries, ", "),
Item: have,
}, nil
}

func diffItemMaps(was, have ItemMap) ([]string, error) {
result := make([]*Diff, 0)
kinds := []RootType{ResourceRootType, DataSourceRootType}
kinds := []RootKind{ResourceRootKind, DataSourceRootKind}
for _, kind := range kinds {
wasItems := was[kind]
haveItems := have[kind]
Expand Down Expand Up @@ -122,7 +120,7 @@ func diffItemMaps(was, have ItemMap) ([]string, error) {
skipPrefix = k
}

change, err := diffItems(kind, wasVal, haveVal)
change, err := diffItems(wasVal, haveVal)
if err != nil {
return nil, fmt.Errorf("failed to compare %s %s: %w", kind, k, err)
}
Expand Down Expand Up @@ -167,18 +165,41 @@ func toMap(item *Item) (map[string]any, error) {
func serializeDiff(list []*Diff) []string {
sort.Slice(list, func(i, j int) bool {
a, b := list[i], list[j]

if a.Item.Root != b.Item.Root {
return a.Item.Root < b.Item.Root
}

if a.Action != b.Action {
return a.Action < b.Action
}

// Resource comes first, then datasource
if a.RootType != b.RootType {
return a.RootType > b.RootType
if a.Item.Path != b.Item.Path {
return a.Item.Path < b.Item.Path
}

if a.Item.Kind != b.Item.Kind {
return a.Item.Kind > b.Item.Kind
}

return a.Item.Path < b.Item.Path
return false
})

// Removes duplicates
unique := make(map[string]*Diff)
for i := 0; i < len(list); i++ {
d := list[i]
k := fmt.Sprintf("%s:%s:%s", d.Action, d.Item.Path, d.Description)
other, ok := unique[k]
if !ok {
unique[k] = d
continue
}
other.AlsoAppliesTo = d.Item
list = append(list[:i], list[i+1:]...)
i--
}

strs := make([]string, len(list))
for i, r := range list {
strs[i] = r.String()
Expand Down
52 changes: 33 additions & 19 deletions changelog/differ_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,102 +12,116 @@ func TestCompare(t *testing.T) {
tests := []struct {
name string
expect string
kind RootType
old, new *Item
}{
{
name: "change enums",
expect: "Change `foo` resource field `bar`: add `foo`, remove `bar`",
kind: ResourceRootType,
expect: "Change `foo` resource field `bar` (enum): add `foo`, remove `bar`",
old: &Item{
Kind: ResourceRootKind,
Type: schema.TypeString,
Path: "foo.bar",
Root: "foo",
Description: "Foo. The possible values are `bar`, `baz`.",
},
new: &Item{
Kind: ResourceRootKind,
Type: schema.TypeString,
Path: "foo.bar",
Root: "foo",
Description: "Foo. The possible values are `foo`, `baz`.",
},
},
{
name: "change enum",
expect: "Change `foo` resource field `bar`: add `foo`",
kind: ResourceRootType,
expect: "Change `foo` resource field `bar` (enum): add `foo`",
old: &Item{
Kind: ResourceRootKind,
Type: schema.TypeString,
Path: "foo.bar",
Root: "foo",
Description: "Foo. The possible values is `bar`",
},
new: &Item{
Kind: ResourceRootKind,
Type: schema.TypeString,
Path: "foo.bar",
Root: "foo",
Description: "Foo. The possible values are `foo`, `bar`.",
},
},
{
name: "add resource field",
expect: "Add `foo` resource field `bar`: Foo",
kind: ResourceRootType,
new: &Item{
Kind: ResourceRootKind,
Type: schema.TypeString,
Path: "foo.bar",
Root: "foo",
Description: "Foo",
},
},
{
name: "remove resource field",
expect: "Remove `foo` resource field `bar`: Foo",
kind: ResourceRootType,
old: &Item{
Kind: ResourceRootKind,
Type: schema.TypeString,
Path: "foo.bar",
Root: "foo",
Description: "Foo",
},
},
{
name: "remove beta from the field",
expect: "Change `foo` resource field `bar`: no longer beta",
kind: ResourceRootType,
old: &Item{
Kind: ResourceRootKind,
Type: schema.TypeString,
Path: "foo.bar",
Root: "foo",
Description: "PROVIDER_AIVEN_ENABLE_BETA",
},
new: &Item{
Kind: ResourceRootKind,
Type: schema.TypeString,
Path: "foo.bar",
Root: "foo",
Description: "Foo",
},
},
{
name: "add beta resource",
expect: "Add `foo` resource _(beta)_: does stuff, PROVIDER_AIVEN_ENABLE_BETA",
kind: ResourceRootType,
new: &Item{
Kind: ResourceRootKind,
Type: schema.TypeString,
Path: "foo",
Root: "foo",
Description: "does stuff, PROVIDER_AIVEN_ENABLE_BETA",
},
},
{
name: "change type",
expect: "Change `foo` resource field `bar`: type ~~`list`~~ → `set`",
kind: ResourceRootType,
old: &Item{
Kind: ResourceRootKind,
Type: schema.TypeList,
Path: "foo.bar",
Root: "foo",
},
new: &Item{
Kind: ResourceRootKind,
Type: schema.TypeSet,
Path: "foo.bar",
Root: "foo",
},
},
}

for _, opt := range tests {
t.Run(opt.name, func(t *testing.T) {
got, err := diffItems(opt.kind, opt.old, opt.new)
got, err := diffItems(opt.old, opt.new)
assert.NoError(t, err)
assert.Equal(t, opt.expect, got.String())
})
Expand All @@ -116,19 +130,19 @@ func TestCompare(t *testing.T) {

func TestSerializeDiff(t *testing.T) {
list := []*Diff{
{Action: AddDiffAction, RootType: ResourceRootType, Description: "foo", Item: &Item{Path: "aiven_opensearch.opensearch_user_config.azure_migration.include_aliases"}},
{Action: ChangeDiffAction, RootType: DataSourceRootType, Description: "remove deprecation", Item: &Item{Path: "aiven_cassandra.cassandra_user_config.additional_backup_regions"}},
{Action: ChangeDiffAction, RootType: ResourceRootType, Description: "remove deprecation", Item: &Item{Path: "aiven_cassandra.cassandra_user_config.additional_backup_regions"}},
{Action: AddDiffAction, RootType: ResourceRootType, Description: "foo", Item: &Item{Path: "aiven_opensearch.opensearch_user_config.s3_migration.include_aliases"}},
{Action: AddDiffAction, RootType: ResourceRootType, Description: "foo", Item: &Item{Path: "aiven_opensearch.opensearch_user_config.gcs_migration.include_aliases"}},
{Action: AddDiffAction, Description: "foo", Item: &Item{Kind: ResourceRootKind, Root: "aiven_opensearch", Path: "aiven_opensearch.opensearch_user_config.azure_migration.include_aliases"}},
{Action: ChangeDiffAction, Description: "remove deprecation", Item: &Item{Kind: DataSourceRootKind, Root: "aiven_cassandra", Path: "aiven_cassandra.cassandra_user_config.additional_backup_regions"}},
{Action: ChangeDiffAction, Description: "remove deprecation", Item: &Item{Kind: ResourceRootKind, Root: "aiven_cassandra", Path: "aiven_cassandra.cassandra_user_config.additional_backup_regions"}},
{Action: AddDiffAction, Description: "foo", Item: &Item{Kind: ResourceRootKind, Root: "aiven_opensearch", Path: "aiven_opensearch.opensearch_user_config.s3_migration.include_aliases"}},
{Action: AddDiffAction, Description: "foo", Item: &Item{Kind: DataSourceRootKind, Root: "aiven_opensearch", Path: "aiven_opensearch.opensearch_user_config.s3_migration.include_aliases"}},
{Action: AddDiffAction, Description: "foo", Item: &Item{Kind: ResourceRootKind, Root: "aiven_opensearch", Path: "aiven_opensearch.opensearch_user_config.gcs_migration.include_aliases"}},
}

expect := []string{
"Change `aiven_cassandra` resource and datasource field `cassandra_user_config.additional_backup_regions`: remove deprecation",
"Add `aiven_opensearch` resource field `opensearch_user_config.azure_migration.include_aliases`: foo",
"Add `aiven_opensearch` resource field `opensearch_user_config.gcs_migration.include_aliases`: foo",
"Add `aiven_opensearch` resource field `opensearch_user_config.s3_migration.include_aliases`: foo",
"Change `aiven_cassandra` resource field `cassandra_user_config.additional_backup_regions`: remove deprecation",
"Change `aiven_cassandra` datasource field `cassandra_user_config.additional_backup_regions`: remove deprecation",
"Add `aiven_opensearch` resource and datasource field `opensearch_user_config.s3_migration.include_aliases`: foo",
}

actual := serializeDiff(list)
Expand Down
10 changes: 7 additions & 3 deletions changelog/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,9 +162,9 @@ func writeChangelog(filePath string, reformat bool, entries []string) error {
func fromProvider(p *schema.Provider) (ItemMap, error) {
// Item names might clash between resources and data sources
// Splits into separate maps
sourceMaps := map[RootType]map[string]*schema.Resource{
ResourceRootType: p.ResourcesMap,
DataSourceRootType: p.DataSourcesMap,
sourceMaps := map[RootKind]map[string]*schema.Resource{
ResourceRootKind: p.ResourcesMap,
DataSourceRootKind: p.DataSourcesMap,
}

items := make(ItemMap)
Expand All @@ -173,7 +173,9 @@ func fromProvider(p *schema.Provider) (ItemMap, error) {
for name, r := range m {
res := &Item{
Name: name,
Root: name,
Path: name,
Kind: kind,
Description: r.Description,
Type: schema.TypeList,
}
Expand All @@ -192,7 +194,9 @@ func fromProvider(p *schema.Provider) (ItemMap, error) {
func walkSchema(name string, this *schema.Schema, parent *Item) []*Item {
item := &Item{
Name: name,
Root: parent.Root,
Path: fmt.Sprintf("%s.%s", parent.Path, name),
Kind: parent.Kind,
ForceNew: this.ForceNew,
Optional: this.Optional,
Sensitive: this.Sensitive,
Expand Down
18 changes: 14 additions & 4 deletions changelog/text.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,21 @@ func removeEnum(text string) string {
var reCode = regexp.MustCompile("`([^`]+)`")

func findEnums(description string) []string {
parts := strings.Split(description, userconfig.PossibleValuesPrefix)
if len(parts) != 2 {
return nil
var values []string
switch {
case strings.Contains(description, userconfig.PossibleValuesPrefix):
// userconfig.PossibleValuesPrefix is used within "internal" pacakge,

Check failure on line 31 in changelog/text.go

View workflow job for this annotation

GitHub Actions / make_lint

`pacakge` is a misspelling of `package` (misspell)
// see userconfig.DescriptionBuilder.PossibleValuesString()
parts := strings.Split(description, userconfig.PossibleValuesPrefix)
if len(parts) != 2 {
return nil
}
values = reCode.FindAllString(parts[1], -1)
case strings.Contains(strings.ToLower(description), "enum"):
// "enum" is used in the user config generator
values = reCode.FindAllString(description, -1)
}
values := reCode.FindAllString(parts[1], -1)

if len(values) == 0 {
return nil
}
Expand Down
Loading

0 comments on commit 2eee614

Please sign in to comment.