Skip to content

Commit

Permalink
Revert "check breakage for adding required fields" (GoogleCloudPlatfo…
Browse files Browse the repository at this point in the history
  • Loading branch information
ScottSuarez authored Oct 2, 2023
1 parent fc4eb6e commit 5a1405f
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 5 deletions.
8 changes: 4 additions & 4 deletions tools/diff-processor/rules/rules_field.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,19 +73,19 @@ func fieldRule_ChangingType_func(old, new *schema.Schema, mc MessageContext) str

var fieldRule_BecomingRequired = FieldRule{
name: "Field becoming Required Field",
definition: "A field cannot become or be added as required. Existing configs may not have this field defined thus breaking them in sequential plan or applies. If you are adding Required to a field so a block won't remain empty, this can cause two issues. First if it's a singular nested field the block may gain more fields later and it's not clear whether the field is actually required so it may be misinterpreted by future contributors. Second if users are defining empty blocks in existing configurations this change will break them. Consider these points in admittance of this type of change.",
definition: "A field cannot become required as existing configs may not have this field defined. Thus, breaking configs in sequential plan or applies. If you are adding Required to a field so a block won't remain empty, this can cause two issues. First if it's a singular nested field the block may gain more fields later and it's not clear whether the field is actually required so it may be misinterpreted by future contributors. Second if users are defining empty blocks in existing configurations this change will break them. Consider these points in admittance of this type of change.",
message: "Field {{field}} changed from optional to required on {{resource}}",
identifier: "field-optional-to-required",
isRuleBreak: fieldRule_BecomingRequired_func,
}

func fieldRule_BecomingRequired_func(old, new *schema.Schema, mc MessageContext) string {
// Ignore for removed fields
if new == nil {
// Ignore for added / removed fields
if old == nil || new == nil {
return ""
}
message := mc.message
if (old == nil || !old.Required) && new.Required {
if !old.Required && new.Required {
return populateMessageContext(message, mc)
}

Expand Down
3 changes: 2 additions & 1 deletion tools/diff-processor/rules/rules_field_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,13 +72,14 @@ var fieldRule_BecomingRequiredTestCases = []fieldTestCase{
expectedViolation: true,
},
{
// TODO: detect this as violation b/300515447
name: "field added as required",
oldField: nil,
newField: &schema.Schema{
Description: "beep",
Required: true,
},
expectedViolation: true,
expectedViolation: false,
},
{
name: "field removed",
Expand Down

0 comments on commit 5a1405f

Please sign in to comment.