From ca3ab5d68ce75337ee52fa7fcbcebcebb59a8b0d Mon Sep 17 00:00:00 2001 From: Reuven Harrison Date: Sun, 21 Jul 2024 10:48:27 +0300 Subject: [PATCH] add attributes (#587) --- checker/api_change.go | 20 +++++++++ checker/attributes_test.go | 76 +++++++++++++++++++++++++++++++++++ checker/change.go | 9 +++++ checker/component_change.go | 2 + checker/config.go | 7 ++++ checker/security_change.go | 2 + data/attributes/base.yaml | 23 +++++++++++ data/attributes/revision.yaml | 24 +++++++++++ docs/ATTRIBUTES.md | 29 +++++++++++++ docs/BREAKING-CHANGES.md | 18 ++++----- formatters/changes.go | 22 +++++----- internal/changelog.go | 2 +- internal/changelog_flags.go | 2 + internal/cmd_flags.go | 3 +- internal/diff.go | 2 +- internal/diff_flags.go | 2 + internal/enum_slice.go | 2 +- internal/flags.go | 14 +++++++ internal/run_test.go | 4 ++ internal/summary.go | 2 +- 20 files changed, 240 insertions(+), 25 deletions(-) create mode 100644 checker/attributes_test.go create mode 100644 data/attributes/base.yaml create mode 100644 data/attributes/revision.yaml create mode 100644 docs/ATTRIBUTES.md diff --git a/checker/api_change.go b/checker/api_change.go index f1f58866..fe4e9e50 100644 --- a/checker/api_change.go +++ b/checker/api_change.go @@ -12,6 +12,8 @@ import ( // ApiChange represnts a change in the Paths Section of an OpenAPI spec type ApiChange struct { + CommonChange + Id string Args []any Comment string @@ -38,7 +40,25 @@ func NewApiChange(id string, config *Config, args []any, comment string, operati Operation: method, Path: path, Source: load.NewSource((*operationsSources)[operation]), + CommonChange: CommonChange{ + Attributes: getAttributes(config, operation), + }, + } +} + +func getAttributes(config *Config, operation *openapi3.Operation) map[string]any { + result := map[string]any{} + for _, tag := range config.Attributes { + if val, ok := operation.Extensions[tag]; ok { + result[tag] = val + } } + + if len(result) == 0 { + return nil + } + + return result } func (c ApiChange) GetSection() string { diff --git a/checker/attributes_test.go b/checker/attributes_test.go new file mode 100644 index 00000000..66dbbf21 --- /dev/null +++ b/checker/attributes_test.go @@ -0,0 +1,76 @@ +package checker_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + "github.com/tufin/oasdiff/checker" + "github.com/tufin/oasdiff/diff" +) + +func TestBreaking_Attributes(t *testing.T) { + s1, err := open("../data/attributes/base.yaml") + require.NoError(t, err) + + s2, err := open("../data/attributes/revision.yaml") + require.NoError(t, err) + + d, osm, err := diff.GetWithOperationsSourcesMap(diff.NewConfig(), s1, s2) + require.NoError(t, err) + errs := checker.CheckBackwardCompatibility(allChecksConfig().WithAttributes([]string{"x-test"}), d, osm) + require.NotEmpty(t, errs) + require.Len(t, errs, 2) + + require.Equal(t, map[string]any{"x-test": []any{"xyz", float64(456)}}, errs[0].GetAttributes()) + require.Equal(t, map[string]any{"x-test": "abc"}, errs[1].GetAttributes()) +} + +func TestBreaking_AttributesNone(t *testing.T) { + s1, err := open("../data/attributes/base.yaml") + require.NoError(t, err) + + s2, err := open("../data/attributes/revision.yaml") + require.NoError(t, err) + + d, osm, err := diff.GetWithOperationsSourcesMap(diff.NewConfig(), s1, s2) + require.NoError(t, err) + errs := checker.CheckBackwardCompatibility(allChecksConfig().WithAttributes([]string{"x-other"}), d, osm) + require.NotEmpty(t, errs) + require.Len(t, errs, 2) + + require.Empty(t, errs[0].GetAttributes()) + require.Empty(t, errs[1].GetAttributes()) +} + +func TestBreaking_AttributesReverse(t *testing.T) { + s1, err := open("../data/attributes/revision.yaml") + require.NoError(t, err) + + s2, err := open("../data/attributes/base.yaml") + require.NoError(t, err) + + d, osm, err := diff.GetWithOperationsSourcesMap(diff.NewConfig(), s1, s2) + require.NoError(t, err) + errs := checker.CheckBackwardCompatibility(allChecksConfig().WithAttributes([]string{"x-test"}), d, osm) + require.NotEmpty(t, errs) + require.Len(t, errs, 2) + + require.Equal(t, map[string]any{"x-test": []any{float64(123), float64(456)}}, errs[0].GetAttributes()) + require.Equal(t, map[string]any{"x-test": float64(123)}, errs[1].GetAttributes()) +} + +func TestBreaking_AttributesTwo(t *testing.T) { + s1, err := open("../data/attributes/base.yaml") + require.NoError(t, err) + + s2, err := open("../data/attributes/revision.yaml") + require.NoError(t, err) + + d, osm, err := diff.GetWithOperationsSourcesMap(diff.NewConfig(), s1, s2) + require.NoError(t, err) + errs := checker.CheckBackwardCompatibility(allChecksConfig().WithAttributes([]string{"x-test", "x-test2"}), d, osm) + require.Len(t, errs, 2) + + require.Equal(t, map[string]any{"x-test": []any{"xyz", float64(456)}}, errs[0].GetAttributes()) + require.Equal(t, map[string]any{"x-test": "abc", "x-test2": "def"}, errs[1].GetAttributes()) +} diff --git a/checker/change.go b/checker/change.go index d478c13e..504744f0 100644 --- a/checker/change.go +++ b/checker/change.go @@ -13,6 +13,7 @@ type Change interface { GetOperationId() string GetPath() string GetSource() string + GetAttributes() map[string]any GetSourceFile() string GetSourceLine() int GetSourceLineEnd() int @@ -22,3 +23,11 @@ type Change interface { SingleLineError(l Localizer, colorMode ColorMode) string MultiLineError(l Localizer, colorMode ColorMode) string } + +type CommonChange struct { + Attributes map[string]any +} + +func (c CommonChange) GetAttributes() map[string]any { + return c.Attributes +} diff --git a/checker/component_change.go b/checker/component_change.go index e4f47b80..62da0915 100644 --- a/checker/component_change.go +++ b/checker/component_change.go @@ -9,6 +9,8 @@ import ( // ComponentChange represnts a change in the Components Section: https://swagger.io/docs/specification/components/ type ComponentChange struct { + CommonChange + Id string Args []any Comment string diff --git a/checker/config.go b/checker/config.go index b8c877a8..daa3a06d 100644 --- a/checker/config.go +++ b/checker/config.go @@ -7,6 +7,7 @@ type Config struct { MinSunsetBetaDays uint MinSunsetStableDays uint LogLevels map[string]Level + Attributes []string } const ( @@ -63,6 +64,12 @@ func (config *Config) WithChecks(checks BackwardCompatibilityChecks) *Config { return config } +// WithAttributes sets a list of attributes to be used. +func (config *Config) WithAttributes(attributes []string) *Config { + config.Attributes = attributes + return config +} + func (config *Config) getLogLevel(checkId string) Level { level, ok := config.LogLevels[checkId] diff --git a/checker/security_change.go b/checker/security_change.go index a1d13adb..df3e5c87 100644 --- a/checker/security_change.go +++ b/checker/security_change.go @@ -9,6 +9,8 @@ import ( // SecurityChange represents a change in the Security Section (not to be confised with components/securitySchemes) type SecurityChange struct { + CommonChange + Id string Args []any Comment string diff --git a/data/attributes/base.yaml b/data/attributes/base.yaml new file mode 100644 index 00000000..6f3616c7 --- /dev/null +++ b/data/attributes/base.yaml @@ -0,0 +1,23 @@ +openapi: 3.0.1 +info: + title: Test API + version: v1 +paths: + /partner-api/test/some-method: + get: + x-test: 123 + tags: + - Test + responses: + "200": + description: Success + /partner-api/test/another-method: + get: + x-test: + - 123 + - 456 + tags: + - Test + responses: + "200": + description: Success diff --git a/data/attributes/revision.yaml b/data/attributes/revision.yaml new file mode 100644 index 00000000..f88cb084 --- /dev/null +++ b/data/attributes/revision.yaml @@ -0,0 +1,24 @@ +openapi: 3.0.1 +info: + title: Test API + version: v1 +paths: + /partner-api/test/some-method: + get: + x-test: "abc" + x-test2: "def" + tags: + - Test + responses: + "201": + description: Success + /partner-api/test/another-method: + get: + x-test: + - "xyz" + - 456 + tags: + - Test + responses: + "201": + description: Success diff --git a/docs/ATTRIBUTES.md b/docs/ATTRIBUTES.md new file mode 100644 index 00000000..6039ca0c --- /dev/null +++ b/docs/ATTRIBUTES.md @@ -0,0 +1,29 @@ +## Adding custom attributes to changelog entries basing on OpenAPI extension tags +Some people annotate their endpoints with OpenAPI Extension tags, for example: +``` +/restapi/oauth/token: + post: + operationId: getToken + x-audience: Public + summary: ... + requestBody: + ... + responses: + ... +``` + +Oasdiff can add these attributes to the changelog in JSON or YAML formats as follows: + +``` +❯ oasdiff changelog base.yaml revision.yaml -f yaml --attributes x-audience +- id: new-optional-request-property + text: added the new optional request property ivr_pin + level: 1 + operation: POST + operationId: getToken + path: /restapi/oauth/token + source: new-revision.yaml + section: paths + attributes: + x-audience: Public +``` \ No newline at end of file diff --git a/docs/BREAKING-CHANGES.md b/docs/BREAKING-CHANGES.md index 6c23f459..59107d8f 100644 --- a/docs/BREAKING-CHANGES.md +++ b/docs/BREAKING-CHANGES.md @@ -101,6 +101,11 @@ This method allows adding new entries to enums used in responses which is very u In most cases the `x-extensible-enum` is similar to enum values, except it allows adding new entries in messages sent to the client (responses or callbacks). If you don't use the `x-extensible-enum` in your OpenAPI specifications, nothing changes for you, but if you do, oasdiff will identify breaking changes related to `x-extensible-enum` parameters and properties. +### Localization +To display changes in other languages, use the `--lang` flag. +Currently English and Russian are supported. +[Please improve oasdiff by adding your own language](https://github.com/Tufin/oasdiff/issues/383). + ### Customizing Severity Levels Oasdiff allows you to change the default severity levels according to your needs. For example, the default severity level of the `api-security-removed` check is `INFO`. You can verify this by running `oasdiff checks`. @@ -113,17 +118,9 @@ Where the file `oasdiff-levels.txt` contains a single line: api-security-removed err ``` -[Here are some examples of breaking and non-breaking changes that oasdiff supports](BREAKING-CHANGES-EXAMPLES.md). -This document is automatically generated from oasdiff unit tests. - -### Localization -To display changes in other languages, use the `--lang` flag. -Currently English and Russian are supported. -[Please improve oasdiff by adding your own language](https://github.com/Tufin/oasdiff/issues/383). - ### Customizing Breaking Changes Checks -If you encounter a change that isn't considered breaking by oasdiff you may: -1. Check if the change is already available as an [optional check](#optional-checks). +If you encounter a change that isn't reported, you may: +1. Run `oasdiff changelog` to see if the check is available as an info-level check, and [customize the level as needed](#customizing-breaking-changes-checks). 2. Add a [custom check](CUSTOMIZING-CHECKS.md) ### Additional Options @@ -133,6 +130,7 @@ If you encounter a change that isn't considered breaking by oasdiff you may: - [Path parameter renaming](PATH-PARAM-RENAME.md) - [Case-insensitive header comparison](HEADER-DIFF.md) - [Comparing multiple specs](COMPOSED.md) +- [Adding OpenAPI Extensions to the changelog output](ATTRIBUTES.md) - [Running from docker](DOCKER.md) - [Embedding in your go program](GO.md) diff --git a/formatters/changes.go b/formatters/changes.go index 7b130fd4..0230646a 100644 --- a/formatters/changes.go +++ b/formatters/changes.go @@ -5,16 +5,17 @@ import ( ) type Change struct { - Id string `json:"id,omitempty" yaml:"id,omitempty"` - Text string `json:"text,omitempty" yaml:"text,omitempty"` - Comment string `json:"comment,omitempty" yaml:"comment,omitempty"` - Level checker.Level `json:"level" yaml:"level"` - Operation string `json:"operation,omitempty" yaml:"operation,omitempty"` - OperationId string `json:"operationId,omitempty" yaml:"operationId,omitempty"` - Path string `json:"path,omitempty" yaml:"path,omitempty"` - Source string `json:"source,omitempty" yaml:"source,omitempty"` - Section string `json:"section,omitempty" yaml:"section,omitempty"` - IsBreaking bool `json:"-" yaml:"-"` + Id string `json:"id,omitempty" yaml:"id,omitempty"` + Text string `json:"text,omitempty" yaml:"text,omitempty"` + Comment string `json:"comment,omitempty" yaml:"comment,omitempty"` + Level checker.Level `json:"level" yaml:"level"` + Operation string `json:"operation,omitempty" yaml:"operation,omitempty"` + OperationId string `json:"operationId,omitempty" yaml:"operationId,omitempty"` + Path string `json:"path,omitempty" yaml:"path,omitempty"` + Source string `json:"source,omitempty" yaml:"source,omitempty"` + Section string `json:"section,omitempty" yaml:"section,omitempty"` + IsBreaking bool `json:"-" yaml:"-"` + Attributes map[string]any `json:"attributes,omitempty" yaml:"attributes,omitempty"` } type Changes []Change @@ -32,6 +33,7 @@ func NewChanges(originalChanges checker.Changes, l checker.Localizer) Changes { OperationId: change.GetOperationId(), Path: change.GetPath(), Source: change.GetSource(), + Attributes: change.GetAttributes(), } } return changes diff --git a/internal/changelog.go b/internal/changelog.go index 0f830699..aba2d777 100644 --- a/internal/changelog.go +++ b/internal/changelog.go @@ -59,7 +59,7 @@ func getChangelog(flags Flags, stdout io.Writer, level checker.Level) (bool, *Re errs, returnErr := filterIgnored( checker.CheckBackwardCompatibilityUntilLevel( - checker.NewConfig(checker.GetAllChecks()).WithOptionalChecks(flags.getIncludeChecks()).WithSeverityLevels(severityLevels).WithDeprecation(flags.getDeprecationDaysBeta(), flags.getDeprecationDaysStable()), + checker.NewConfig(checker.GetAllChecks()).WithOptionalChecks(flags.getIncludeChecks()).WithSeverityLevels(severityLevels).WithDeprecation(flags.getDeprecationDaysBeta(), flags.getDeprecationDaysStable()).WithAttributes(flags.getAttributes()), diffResult.diffReport, diffResult.operationsSources, level), diff --git a/internal/changelog_flags.go b/internal/changelog_flags.go index b2189ea5..2ce0a606 100644 --- a/internal/changelog_flags.go +++ b/internal/changelog_flags.go @@ -6,6 +6,8 @@ import ( ) type ChangelogFlags struct { + CommonFlags + base *load.Source revision *load.Source composed bool diff --git a/internal/cmd_flags.go b/internal/cmd_flags.go index 66442963..8edd434e 100644 --- a/internal/cmd_flags.go +++ b/internal/cmd_flags.go @@ -55,11 +55,12 @@ func addCommonBreakingFlags(cmd *cobra.Command, flags Flags) { enumWithOptions(cmd, newEnumValue(localizations.GetSupportedLanguages(), localizations.LangDefault, flags.refLang()), "lang", "l", "language for localized output") cmd.PersistentFlags().StringVarP(flags.refErrIgnoreFile(), "err-ignore", "", "", "configuration file for ignoring errors") cmd.PersistentFlags().StringVarP(flags.refWarnIgnoreFile(), "warn-ignore", "", "", "configuration file for ignoring warnings") - cmd.PersistentFlags().VarP(newEnumSliceValue(checker.GetOptionalRuleIds(), nil, flags.refIncludeChecks()), "include-checks", "i", "comma-separated list of optional checks") + cmd.PersistentFlags().VarP(newEnumSliceValue(checker.GetOptionalRuleIds(), nil, flags.refIncludeChecks()), "include-checks", "i", "optional checks") hideFlag(cmd, "include-checks") cmd.PersistentFlags().UintVarP(flags.refDeprecationDaysBeta(), "deprecation-days-beta", "", checker.DefaultBetaDeprecationDays, "min days required between deprecating a beta resource and removing it") cmd.PersistentFlags().UintVarP(flags.refDeprecationDaysStable(), "deprecation-days-stable", "", checker.DefaultStableDeprecationDays, "min days required between deprecating a stable resource and removing it") enumWithOptions(cmd, newEnumValue([]string{"auto", "always", "never"}, "auto", flags.refColor()), "color", "", "when to colorize textual output") enumWithOptions(cmd, newEnumValue(formatters.SupportedFormatsByContentType(formatters.OutputChangelog), string(formatters.FormatText), flags.refFormat()), "format", "f", "output format") cmd.PersistentFlags().StringVarP(flags.refSeverityLevelsFile(), "severity-levels", "", "", "configuration file for custom severity levels") + cmd.PersistentFlags().StringSliceVarP(flags.refAttributes(), "attributes", "", nil, "OpenAPI Extensions to include in json or yaml output") } diff --git a/internal/diff.go b/internal/diff.go index 3ec6bd0b..c22fed37 100644 --- a/internal/diff.go +++ b/internal/diff.go @@ -26,7 +26,7 @@ func getDiffCmd() *cobra.Command { } addCommonDiffFlags(&cmd, &flags) - enumWithOptions(&cmd, newEnumSliceValue(diff.ExcludeDiffOptions, nil, flags.refExcludeElements()), "exclude-elements", "e", "comma-separated list of elements to exclude") + enumWithOptions(&cmd, newEnumSliceValue(diff.ExcludeDiffOptions, nil, flags.refExcludeElements()), "exclude-elements", "e", "elements to exclude") enumWithOptions(&cmd, newEnumValue(formatters.SupportedFormatsByContentType(formatters.OutputDiff), string(formatters.FormatYAML), &flags.format), "format", "f", "output format") cmd.PersistentFlags().BoolVarP(&flags.failOnDiff, "fail-on-diff", "o", false, "exit with return code 1 when any change is found") diff --git a/internal/diff_flags.go b/internal/diff_flags.go index 49009410..c6fad9fc 100644 --- a/internal/diff_flags.go +++ b/internal/diff_flags.go @@ -6,6 +6,8 @@ import ( ) type DiffFlags struct { + CommonFlags + base *load.Source revision *load.Source composed bool diff --git a/internal/enum_slice.go b/internal/enum_slice.go index e1d2eb95..271d46e7 100644 --- a/internal/enum_slice.go +++ b/internal/enum_slice.go @@ -97,7 +97,7 @@ func (s *enumSliceValue) Set(val string) error { } func (s *enumSliceValue) Type() string { - return "csv" + return "strings" } func (s *enumSliceValue) String() string { diff --git a/internal/flags.go b/internal/flags.go index e5a97e80..c1b9f6fe 100644 --- a/internal/flags.go +++ b/internal/flags.go @@ -27,6 +27,7 @@ type Flags interface { getFailOnDiff() bool getAsymmetric() bool getSeverityLevelsFile() string + getAttributes() []string setBase(source *load.Source) setRevision(source *load.Source) @@ -54,4 +55,17 @@ type Flags interface { refDeprecationDaysStable() *uint refColor() *string refSeverityLevelsFile() *string + refAttributes() *[]string +} + +type CommonFlags struct { + attributes []string +} + +func (flags *CommonFlags) getAttributes() []string { + return flags.attributes +} + +func (flags *CommonFlags) refAttributes() *[]string { + return &flags.attributes } diff --git a/internal/run_test.go b/internal/run_test.go index fe667bc0..abfc18d0 100644 --- a/internal/run_test.go +++ b/internal/run_test.go @@ -247,6 +247,10 @@ func Test_Changelog(t *testing.T) { require.Len(t, cl, 1) } +func Test_ChangelogWithAttributes(t *testing.T) { + require.Zero(t, internal.Run(cmdToArgs("oasdiff changelog ../data/openapi-test1.yaml ../data/openapi-test3.yaml --attributes x-beta,x-extension-test -f yaml"), io.Discard, io.Discard)) +} + func Test_BreakingChangesChangelogOptionalCheckersAreInfoLevel(t *testing.T) { var stdout bytes.Buffer require.Zero(t, internal.Run(cmdToArgs("oasdiff changelog ../data/run_test/changelog_include_checks_base.yaml ../data/run_test/changelog_include_checks_revision.yaml --format json"), &stdout, io.Discard)) diff --git a/internal/summary.go b/internal/summary.go index ce6649bf..e05d7474 100644 --- a/internal/summary.go +++ b/internal/summary.go @@ -23,7 +23,7 @@ func getSummaryCmd() *cobra.Command { } addCommonDiffFlags(&cmd, &flags) - enumWithOptions(&cmd, newEnumSliceValue(diff.ExcludeDiffOptions, nil, flags.refExcludeElements()), "exclude-elements", "e", "comma-separated list of elements to exclude") + enumWithOptions(&cmd, newEnumSliceValue(diff.ExcludeDiffOptions, nil, flags.refExcludeElements()), "exclude-elements", "e", "elements to exclude") enumWithOptions(&cmd, newEnumValue(formatters.SupportedFormatsByContentType(formatters.OutputSummary), string(formatters.FormatYAML), &flags.format), "format", "f", "output format") cmd.PersistentFlags().BoolVarP(&flags.failOnDiff, "fail-on-diff", "", false, "exit with return code 1 when any change is found")