Improve Cloudfront Configuration

[ezgidemirel]

Description of your changes

In this change, we are:

• solving infinite update loop issue
• late initializing the elements of "Items" array
• deducing "quantity" field from length of the "Items" array

Please note that, this change causes API changes. It removes quantity fields from the CRD.

Fixes #1108 #789 #1174

I have:

• Read and followed Crossplane's contribution process.
• Run make reviewable test to ensure this PR is ready for review.

How has this code been tested

• Created a Cloudfront distribution with the following manifest:

apiVersion: cloudfront.aws.crossplane.io/v1alpha1
kind: Distribution
metadata:
  name: ezgi
spec:
  forProvider:
    region: us-east-1
    distributionConfig:
      enabled: true
      comment: Example CloudFront Distribution 3
      priceClass: PriceClass_100 
      origins:
        items:
          - domainName: app.my.cloud
            id: customOrigin
            customOriginConfig:
              httpPort: 80
              originProtocolPolicy: "https-only"
              originReadTimeout: 10
              originKeepaliveTimeout: 5
              httpSPort: 443
              originSSLProtocols:
                items:
                  - TLSv1.2
      cacheBehaviors:
        items:
        - pathPattern: /*/services-email-validation-api/*          
          targetOriginID: customOrigin
          compress: false
          viewerProtocolPolicy: https-only
          allowedMethods:        
            items: 
            - GET 
            - HEAD 
            - OPTIONS
            - PUT
            - POST
            - PATCH
            - DELETE
          minTTL: 0
          maxTTL: 0
          defaultTTL: 0
          forwardedValues:
            queryString: true         
            cookies:
              forward: all
            headers:
              items:
              - Accept
              - Accept-Charset
              - Accept-Encoding
              - Accept-Language
              - Authorization
              - Accept-Datetime
              - Referer
              - Origin
      defaultCacheBehavior:
        targetOriginID: customOrigin
        compress: false
        viewerProtocolPolicy: https-only
        allowedMethods:    
          items:
          - GET 
          - HEAD 
          - OPTIONS
          - PUT
          - POST
          - PATCH
          - DELETE
        minTTL: 0
        maxTTL: 0
        defaultTTL: 0
        forwardedValues:
          queryString: true
          cookies:
            forward: all
          headers:
            items:
            - Accept
            - Accept-Charset
            - Accept-Encoding
            - Accept-Language
            - Authorization
            - Accept-Datetime
            - Referer
            - Origin

• Updated spec.forProvider.distributionConfig.cacheBehaviors.items[0].allowedMethods.cachedMethods as below:

ky distribution.cloudfront.aws.crossplane.io/ezgi -o jsonpath='{.spec.forProvider.distributionConfig.cacheBehaviors.items[0].allowedMethods.cachedMethods}'
{"items":["HEAD","GET"]}

kubectl get managed                                                                Ezgis-MacBook-Pro.local: Wed Mar  2 15:46:54 2022

Warning: Please use v1beta1 version of SNS group.
NAME                                             READY   SYNCED   EXTERNAL-NAME
distribution.cloudfront.aws.crossplane.io/ezgi   True    True     E3B8N1APYMYPBX

NAME                                      READY   SYNCED   AGE
bucket.s3.aws.crossplane.io/ezgi-bucket   True    True     7d22h

• Deleted the resource

[haarchri]

@ezgidemirel thanks for this enhancement - does this also fixes #1174 ?

[ezgidemirel]

Hi @haarchri, thanks for mentioning the issue! I created a resource with following CacheBehaviors configuration and it became ready.

cacheBehaviors:
  items:
  - pathPattern: /images/*
    targetOriginID: s3Origin
    viewerProtocolPolicy: https-only
    minTTL: 0
    forwardedValues:
      cookies:
        forward: none
        queryString: false

Rest of the fields are late initialized with the default values coming from AWS:

      cacheBehaviors:
        items:
        - allowedMethods:
            cachedMethods:
              items:
              - HEAD
              - GET
            items:
            - HEAD
            - GET
          compress: false
          defaultTTL: 86400
          fieldLevelEncryptionID: ""
          forwardedValues:
            cookies:
              forward: none
            headers: {}
            queryString: false
            queryStringCacheKeys: {}
          lambdaFunctionAssociations: {}
          maxTTL: 31536000
          minTTL: 0
          pathPattern: /images/*
          smoothStreaming: false
          targetOriginID: s3Origin
          viewerProtocolPolicy: https-only

Please note that, viewerProtocolPolicy, minTTL and forwardedValues fields are required.

[haarchri]

LGTM - tested the implementation with the following issues: #1108 #789 and #1174

[SAEb-ai]

Hi @muvaf!! I am interested in applying for Report breaking changes in CRD schemas for Pull Requests project of Crosspalne under the summer cohort of LFX mentorship program, 2022.

It took a lot out of me to come to those PR's that were labelled as breaking-change to understand what changes are actually considered as breaking. Here are a few thoughts that had came to my mind after seeing some discussions in the PR's that are marked as breaking-change:

• When there is a change in group of spec in CRD (DynamoDB services are generated by ACK #446 (comment)), consider that change to be breaking.
• When there is change in a field in openAPIV3Schema i.e when a particular field is deleted or the type and format of the particular field are changed( File), consider that change to be breaking.

I just wanted to confirm from you whether my understanding of the project related to breaking changes is going in the right direction or not? Also, it would be really helpful for me in understanding the project more thoroughly if you could please state some more points that the whole community considers for breaking-changes.

[muvaf]

Hi @SAEb-ai ! I'd like to reiterate what I mentioned in the issue, please do not start any work/design before acceptance since we reserved that issue to the selected mentee, so your work may end up being not merged.

Regarding your findings, yes these are considered as breaking changes. You can also look up what's considered as breaking change in OpenAPI format as that's a widely accepted standard.

[SAEb-ai]

Hi @SAEb-ai ! I'd like to reiterate what I mentioned crossplane/crossplane#2863 (comment) please do not start any work/design before acceptance since we reserved that issue to the selected mentee, so your work may end up being not merged.

Yep. I have seen that. But I am not preparing any design doc. I am just trying to understand the project thoroughly which in turn would help me in writing the cover letter in a more verbose way.

Regarding your findings, yes these are considered as breaking changes. You can also look up what's considered as breaking change in OpenAPI format as that's a widely accepted standard.

Thanks @muvaf for specifying more pointers. I got my answer.