Skip to content

Commit

Permalink
CLOUDP-277156: [API Platform] Validate exemptions (#309)
Browse files Browse the repository at this point in the history
  • Loading branch information
andreaangiolillo authored Dec 12, 2024
1 parent 6db8694 commit c22eebe
Show file tree
Hide file tree
Showing 5 changed files with 106 additions and 1 deletion.
28 changes: 28 additions & 0 deletions tools/cli/internal/breakingchanges/exemptions.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package breakingchanges

import (
Expand Down Expand Up @@ -54,6 +55,29 @@ func isWithinExpirationDate(exemption Exemption) bool {
return exemptUntil.After(date) || exemptUntil.Equal(date)
}

func validateExemption(exemption Exemption) error {
if _, err := time.Parse("2006-01-02", exemption.ExemptUntil); err != nil {
return fmt.Errorf("validation error: %v. Exemption: %s", err, exemption)
}

if err := validateField(exemption.Reason, "reason", exemption); err != nil {
return err
}

if err := validateField(exemption.BreakingChangeDescription, "breaking_change_description", exemption); err != nil {
return err
}

return validateField(exemption.ExemptUntil, "exempt_until", exemption)
}

func validateField(fieldValue, fieldName string, exemption Exemption) error {
if fieldValue == "" {
return fmt.Errorf("validation error: empty value for the '%s' field is not allowed. Exemption: '%s'", fieldName, exemption)
}
return nil
}

func transformComponentEntry(breakingChangeDescription string) string {
if strings.Contains(breakingChangeDescription, "api-schema-removed") && !strings.Contains(breakingChangeDescription, "in components") {
return fmt.Sprintf("in components %s", breakingChangeDescription)
Expand Down Expand Up @@ -85,6 +109,10 @@ func GetValidExemptionsList(exemptionsPath string, ignoreExpiration bool, fs afe

var validExemptions []Exemption
for _, exemption := range exemptions {
if err := validateExemption(exemption); err != nil {
return nil, err
}

if ignoreExpiration || isWithinExpirationDate(exemption) {
validExemptions = append(validExemptions, exemption)
}
Expand Down
75 changes: 75 additions & 0 deletions tools/cli/internal/breakingchanges/exemptions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,3 +118,78 @@ func TestGenerateExemptionsFileWithFs(t *testing.T) {
assert.Equal(t, expectedContent, string(data))
})
}

func TestValidateExemption(t *testing.T) {
tests := []struct {
name string
exemption Exemption
expectedError require.ErrorAssertionFunc
}{
{
name: "Valid exemption",
exemption: Exemption{
Reason: "Some reason",
BreakingChangeDescription: "Description of breaking change",
HideFromChangelog: "false",
ExemptUntil: "2024-12-11",
},
expectedError: require.NoError,
},
{
name: "Invalid date format",
exemption: Exemption{
Reason: "Some reason",
BreakingChangeDescription: "Description of breaking change",
HideFromChangelog: "false",
ExemptUntil: "invalid-date",
},
expectedError: require.Error,
},
{
name: "Empty Reason field",
exemption: Exemption{
Reason: "",
BreakingChangeDescription: "Description of breaking change",
HideFromChangelog: "false",
ExemptUntil: "2024-12-11",
},
expectedError: require.Error,
},
{
name: "Empty BreakingChangeDescription field",
exemption: Exemption{
Reason: "Some reason",
BreakingChangeDescription: "",
HideFromChangelog: "false",
ExemptUntil: "2024-12-11",
},
expectedError: require.Error,
},
{
name: "Empty HideFromChangelog field",
exemption: Exemption{
Reason: "Some reason",
BreakingChangeDescription: "Description of breaking change",
HideFromChangelog: "",
ExemptUntil: "2024-12-11",
},
expectedError: require.NoError,
},
{
name: "Empty ExemptUntil field",
exemption: Exemption{
Reason: "Some reason",
BreakingChangeDescription: "Description of breaking change",
HideFromChangelog: "false",
ExemptUntil: "",
},
expectedError: require.Error,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
tt.expectedError(t, validateExemption(tt.exemption))
})
}
}
2 changes: 1 addition & 1 deletion tools/cli/internal/cli/breakingchanges/exemptions/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func (o *Opts) PreRunE(_ []string) error {
return nil
}

// Builder builds the merge command with the following signature:
// ParseBuilder builds the merge command with the following signature:
// breaking-changes exemptions parse -e file_path
func ParseBuilder() *cobra.Command {
opts := &Opts{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1717,6 +1717,7 @@
"hide_from_changelog": "true"
- "breaking_change_description": "DELETE /api/atlas/v2/orgs/{orgId}/serviceAccounts/{serviceAccountId}/secrets/{secretId} api path removed without deprecation [api-path-removed-without-deprecation]"
"exempt_until": "2024-06-01"
"reason": "Spec correction"
- "breaking_change_description": "removed the schema 'DiskBackupBaseRestoreMember' [api-schema-removed]"
"exempt_until": "2024-05-30"
"reason": "Spec correction"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1717,6 +1717,7 @@
"hide_from_changelog": "true"
- "breaking_change_description": "DELETE /api/atlas/v2/orgs/{orgId}/serviceAccounts/{serviceAccountId}/secrets/{secretId} api path removed without deprecation [api-path-removed-without-deprecation]"
"exempt_until": "2024-06-01"
"reason": "Spec correction"
- "breaking_change_description": "removed the schema 'DiskBackupBaseRestoreMember' [api-schema-removed]"
"exempt_until": "2024-05-30"
"reason": "Spec correction"
Expand Down

0 comments on commit c22eebe

Please sign in to comment.