Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

incorrect handling of multiple behavior or criteria with duplicate names #56

Closed
sbfaulkner opened this issue Oct 31, 2019 · 7 comments
Closed

incorrect handling of multiple behavior or criteria with duplicate names #56

sbfaulkner opened this issue Oct 31, 2019 · 7 comments

Comments

@sbfaulkner
Copy link

sbfaulkner commented Oct 31, 2019 •
edited
Loading

I'm using terraform to provision a property, and ran into a case where it drops behaviors (and criteria).

It happens whenever the "name" is the same for two behaviours (or two criteria) within the same rule.

eg.

if I terraform a property with the following rule...

{
    "name": "Restrict Method",
    "children": [],
    "behaviors": [
        {
            "name": "constructResponse",
            "options": {
                "enabled": true,
                "body": "<html>\n  <head>\n    <title>405 Method Not Allowed.</title>\n  </head>\n  <body>\n    <h1>Error 405 Method Not Allowed.</h1>\n    <p>Method Not Allowed.</p>\n  </body>\n</html>",
                "responseCode": 405,
                "forceEviction": false
            }
        }
    ],
    "criteria": [
        {
            "name": "requestMethod",
            "options": {
                "matchOperator": "IS_NOT",
                "value": "GET"
            }
        },
        {
            "name": "requestMethod",
            "options": {
                "matchOperator": "IS_NOT",
                "value": "HEAD"
            }
        },
        {
            "name": "requestMethod",
            "options": {
                "matchOperator": "IS_NOT",
                "value": "OPTIONS"
            }
        }
    ],
    "criteriaMustSatisfy": "all",
    "comments": "Only allow GET, HEAD and OPTIONS"
}

I get a property with the rule below (ie. only the last criteria)...

{
    "name": "Restrict Method",
    "children": [],
    "behaviors": [
        {
            "name": "constructResponse",
            "options": {
                "enabled": true,
                "body": "<html>\n  <head>\n    <title>405 Method Not Allowed.</title>\n  </head>\n  <body>\n    <h1>Error 405 Method Not Allowed.</h1>\n    <p>Method Not Allowed.</p>\n  </body>\n</html>",
                "responseCode": 405,
                "forceEviction": false
            }
        }
    ],
    "criteria": [
        {
            "name": "requestMethod",
            "options": {
                "matchOperator": "IS_NOT",
                "value": "OPTIONS"
            }
        }
    ],
    "criteriaMustSatisfy": "all",
    "comments": "Only allow GET, HEAD and OPTIONS"
}

This is be a bug in this underlying go package... ie. this code replaces duplicate names instead of adding them...

https://github.com/akamai/AkamaiOPEN-edgegrid-golang/blob/master/papi-v1/rules.go#L221-L277

While the tests indicate this is intended behaviour, it doesn't make sense for behaviors or criteria, since names can be duplicates by design.
@sbfaulkner sbfaulkner changed the title possible bug when handling multiple behavior and criteria incorrection handling of multiple behavior or criteria with duplicate names Nov 8, 2019
@sbfaulkner sbfaulkner changed the title incorrection handling of multiple behavior or criteria with duplicate names incorrect handling of multiple behavior or criteria with duplicate names Nov 8, 2019
@martinstibbe
Copy link
Contributor

I was able to duplicate this and can support having criteria as duplicates properly stored with a fix to library do you have an example of duplicate behaviors I can test with ?

@sbfaulkner
Copy link
Author

sbfaulkner commented Nov 22, 2019 •
edited
Loading

@martinstibbe here's one...

{
    "name": "Modify cache key",
    "children": [],
    "behaviors": [
        {
            "name": "cacheId",
            "options": {
                "rule": "INCLUDE_VARIABLE",
                "variableName": "PMUSER_WEBP"
            }
        },
        {
            "name": "cacheId",
            "options": {
                "rule": "INCLUDE_ALL_QUERY_PARAMS"
            }
        }
    ],
    "criteria": [],
    "criteriaMustSatisfy": "all",
    "comments": ""
}

@martinstibbe
Copy link
Contributor

#59
This has changes that work with your examples using Terraform provider to create them with duplicate behaviors and criteria will merge once one last travis test corrected

@martinstibbe
Copy link
Contributor

Merged into master and tagged as v.0.9.2

@sbfaulkner
Copy link
Author

thanks @martinstibbe -- are you a maintainer for the Terraform provider too so we can get a release cut using this version of the golang pkg?

@martinstibbe
Copy link
Contributor

martinstibbe commented Nov 25, 2019 via email

@sbfaulkner sbfaulkner mentioned this issue Nov 28, 2019
Closed
@martinstibbe
Copy link
Contributor

Thanks for your help in testing this one.
Release 0.1.4 https://releases.hashicorp.com/terraform-provider-akamai/0.1.4/ seems to work with this library fix in place

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@sbfaulkner @martinstibbe