feat(Conditions): add support for Conditions on API event resources

[keetonian]

Issue #, if available:
#758

Description of changes:
Add ability to honor conditions placed on functions with API events.

Testing:
Templates were generated using SAM locally and deployed via CFN to test parts of this implementation. I did this with the normal and CORS templates; I have not tested the Auth example (should work the same).

Note to reviewers:
This PR is very long, mostly because I am testing several things related to APIs. The actual code difference is about ~170 lines, all the rest are for tests. The output files for these templates with APIs are very large and hard to read; I have run the translator on similar templates and verified that they deploy to CFN.

Callout:
I have not tested to see if CFN will allow SAM to create extra Conditions. I assume that this is something that is allowed by CFN.

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

[keetonian]

I think I need to add a few more unit tests, but this contains all the implementation as well as end-to-end tests that show this feature.

[codecov-io]

Codecov Report

Merging #804 into release/v1.10.0 will decrease coverage by <.01%.
The diff coverage is 95.08%.

@@                 Coverage Diff                 @@
##           release/v1.10.0     #804      +/-   ##
===================================================
- Coverage            94.54%   94.54%   -0.01%     
===================================================
  Files                   67       67              
  Lines                 2696     2824     +128     
  Branches               481      508      +27     
===================================================
+ Hits                  2549     2670     +121     
- Misses                  77       81       +4     
- Partials                70       73       +3

Impacted Files	Coverage Δ
samtranslator/model/eventsources/push.py	88.73% <100%> (+0.15%)	⬆️
samtranslator/sdk/resource.py	100% <100%> (ø)	⬆️
samtranslator/plugins/api/implicit_api_plugin.py	96.26% <91.93%> (-3.74%)	⬇️
samtranslator/model/intrinsics.py	94.44% <93.75%> (-1.02%)	⬇️
samtranslator/swagger/swagger.py	98.32% <97.22%> (+0.39%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 27c77db...55e2ee0. Read the comment docs.

[brettstack]

This assumes the method is in this position, but this is not always true. For example, I could have a condition that when evaluated to true returns NoValue and when false returns the method.

[keetonian]

Good point. SAM doesn't set something like that up, but someone writing Swagger could.

[keetonian]

This is a very good callout. Will have to think about this.

[keetonian]

Ok. Couple of thoughts here.

This has never worked up to this point, which means that no existing SAM templates will have a conditional inside of a method configuration. So, whatever we do here won't break any existing customers, but could potentially affect future uses of SAM.

SAM uses this code, I think, only for two things:

1. Creating Swagger for the user (generating APIGW paths and methods)
2. Adding an authorizer to a method.

This means that the only time (at least as SAM is now) a customer would have SAM read their swagger is when they are using custom authorizers. We should still assume that other use cases could arise.

Possible solutions:

1. Leave as is:
   1. Document that SAM only modifies the first element of a Condition that is nested under a method.
   2. This means that, with authorizers, SAM would only add an authorizer to the first item in the conditional and ignore the second entirely.
   3. SAM will also throw an error if the first item is invalid.
2. Check the first item to see if it is a valid path, and, if not, return the second item.
   1. This is an either/or situation. It still wouldn't handle the case where both paths would need to be modified.
3. Just do whatever we're doing to both items in the list, and throw an error only if both of them are invalid paths.
   1. Might be the safest approach.
   2. Would a customer ever want SAM to only modify the first one and not touch the second?
   3. Or should we return both parts of this conditional list and make sure that it is a valid path before modifying it (for authorizers or other similar things we do in the future)?

[keetonian]

The third option might be the best option here, should we go ahead and modify all valid method configurations that are in the conditional list? The change is pretty small I think

[brettstack]

3rd option sounds the best

[keetonian]

Ok. Several things need to happen that come out of a discussion about paths.

Problem Description:
In the current implementation, APIGW methods are added based on the conditions on their underlying functions. However, this can leave orphaned paths with no attached methods to them, which doesn't seem like what we want to do.

Example:

Solution:
We want to add a condition to the paths much like we add a condition to the implicit API, which condition is either:

• the condition on the method(s) if only one condition is present
• an aggregate of all conditions on the methods inside of that condition
• no condition if any method doesn't have a condition attached to it.

Example JSON:

The "" should be replaced with the name given to the condition that is created that contains both "Cond" and "MyCondition" conditions that are found on the methods inside of the "/sub" path.

"paths": {
  "Fn::If": [
    "<GeneratedPathConditionName>",
    {
      "/sub": {
        "post": {
          "Fn::If": [
            "Cond",
            {
              "x-amazon-apigateway-integration": {
                "httpMethod": "POST",
                "type": "aws_proxy",
                "uri": {
                  "Fn::If": [
                    "Cond",
                    {
                      "Fn::Sub": "arn:aws:apigateway:${AWS::Region}:lambda:path/2015-03-31/functions/${helloworld.Arn}/invocations"
                    },
                    {
                      "Ref": "AWS::NoValue"
                    }
                  ]
                }
              },
              "responses": {}
            },
            {
              "Ref": "AWS::NoValue"
            }
          ]
        },
        "get": {
          "Fn::If": [
            "MyCondition",
            {
              "x-amazon-apigateway-integration": {
                "httpMethod": "POST",
                "type": "aws_proxy",
                "uri": {
                  "Fn::If": [
                    "MyCondition",
                    {
                      "Fn::Sub": "arn:aws:apigateway:${AWS::Region}:lambda:path/2015-03-31/functions/${hello.Arn}/invocations"
                    },
                    {
                      "Ref": "AWS::NoValue"
                    }
                  ]
                }
              },
              "responses": {}
            },
            {
              "Ref": "AWS::NoValue"
            }
          ]
        }
      }
    },
    {
      "Ref": "AWS::NoValue"
    },
  ]
}

Implementation Ideas:

I don't really know how to make this work well with generated swagger on an explicit API, but for implicit APIs it would be much easier. You would aggregate conditions, as is currently done, but do it in a dictionary of {path_name : set(conditions)}. Explicit APIs will require more thought, and this might need to be done inside the swagger logic itself (which would work as well for Implicit APIs as well, keeping the implementation for both of them the same). However, I don't see a way to add a Condition to the template from where the swagger logic is run, so this may end up requiring a plugin or other method, or passing the Conditions dictionary into the model (as a key word arg) so it can be modified.

Necessary Updates:

Swagger tests:

• https://github.com/keetonian/serverless-application-model/blob/release/v1.10.0-api-event/tests/swagger/test_swagger.py#L95
• https://github.com/keetonian/serverless-application-model/blob/release/v1.10.0-api-event/tests/swagger/test_swagger.py#L260

All new API integration tests (included inside of this PR)

[jlhood]

Approving since I'm a reviewer on this. However, Keeton reviewed my commits and gave me verbal approval.