Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

check breakage for adding required fields #9126

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 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.",
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.",
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 added / removed fields
if old == nil || new == nil {
// Ignore for removed fields
if new == nil {
return ""
}
message := mc.message
if !old.Required && new.Required {
if (old == nil || !old.Required) && new.Required {
return populateMessageContext(message, mc)
}

Expand Down
3 changes: 1 addition & 2 deletions tools/diff-processor/rules/rules_field_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,14 +72,13 @@ 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: false,
expectedViolation: true,
},
{
name: "field removed",
Expand Down