Skip to content

feat: support openapi yaml include validation#3451

Merged
hawflau merged 9 commits intoaws:developfrom
NickHeap2:develop
Jan 21, 2022
Merged

feat: support openapi yaml include validation#3451
hawflau merged 9 commits intoaws:developfrom
NickHeap2:develop

Conversation

@NickHeap2
Copy link
Contributor

@NickHeap2 NickHeap2 commented Nov 7, 2021

Which issue(s) does this change fix?

#3041

Why is this change necessary?

If you are using OpenAPI yaml specifications it makes sense to use them for the API DefinitionBody like this:

      DefinitionBody:
        'Fn::Transform':
          Name: AWS::Include
          Parameters:
            Location: openapi.yaml

Whilst this is valid in a template it is currently not supported by sam validate.

How does it address the issue?

This change detects this pattern and replaces the DefinitionBody with the actual contents of the file allowing the validation to succeed.

What side effects does this change have?

Potentially someone could be relying on the existing behaviour where it fails for this pattern to prevent that pattern being used.

Checklist

  • Add input/output type hints to new functions/methods
  • Write design document (Do I need to write a design document?)
  • Write unit tests
  • Write/update functional tests
  • Write/update integration tests
  • make pr passes
  • make update-reproducible-reqs if dependencies were changed
  • Write documentation

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@NickHeap2 NickHeap2 closed this Nov 7, 2021
@NickHeap2 NickHeap2 changed the title Support openapi yaml include validation feat: support openapi yaml include validation Nov 7, 2021
@NickHeap2 NickHeap2 reopened this Nov 7, 2021
@NickHeap2 NickHeap2 marked this pull request as ready for review November 7, 2021 22:06
@github-actions github-actions bot added the area/validate sam validate command label Nov 11, 2021
Copy link
Contributor

@jfuss jfuss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the contribution!

Just a couple small comments. If you could address those, that would be awesome.

@NickHeap2
Copy link
Contributor Author

@jfuss any chance of getting this across the line?

@jfuss
Copy link
Contributor

jfuss commented Jan 20, 2022

@NickHeap2 Apologizes on the slow response here. I was on parental leave.

I am getting back and collecting the PRs I need to respond too and looking to scope time in the coming weeks to tackle them in batches.

Copy link
Contributor

@jfuss jfuss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks for the contribution @NickHeap2

I will find another member of the team to review as well (as we required two approvals).

@hawflau hawflau merged commit f48262d into aws:develop Jan 21, 2022
mildaniel added a commit that referenced this pull request Feb 23, 2022
@mingkun2020
Copy link
Contributor

Hi @NickHeap2, we have to revert the changes due to the bug reported in this issue#3676 by PR#3685. The validation failed when the location is an intrinsic function like Location: !Join ["", ["s3://", !Ref SomePath, "/openapi.yaml"]].

@NickHeap2
Copy link
Contributor Author

OK @mingkun2020 I'll get the change reworked as soon as I get a chance.

mildaniel added a commit that referenced this pull request Feb 23, 2022
mndeveci pushed a commit to mndeveci/aws-sam-cli that referenced this pull request Apr 5, 2022
* feat: allow include of openapi definition to pass validation

* fix: type error

* chore: increase unit test coverage

* chore: black format change

* chore: add more tests

* fix: cleanup for pr

Change to use pathlib.
Move debug log statement to before next statement.
Remove unused test file.

* fix: use mocks instead of file system

* feat: integration tests

fix: add debug message when import file can't be found

Co-authored-by: Wing Fung Lau <4760060+hawflau@users.noreply.github.com>
mndeveci pushed a commit to mndeveci/aws-sam-cli that referenced this pull request Apr 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/validate sam validate command pr/external

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants