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.

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

convert all underscore fields to camel case #235

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

headyj
Copy link

@headyj headyj commented Jan 15, 2020

Fixing issue #119

@ghost ghost added the size/XS label Jan 15, 2020
@papineni87
Copy link
Collaborator

Looks like acceptance test is failing for this change

BIGIP_USER=admin BIGIP_PASSWORD=xxxxx BIGIP_HOST=10.145.69.41 TF_ACC=1 go test ./bigip -v -run=TestAccBigipLtmPolicy_create
--- FAIL: TestAccBigipLtmPolicy_create (0.87s)
testing.go:569: Step 0 error: errors during apply:

    Error: 010716de:3: Policy '/Common/Drafts/test-policy', rule 'rule6'; target 'forward' action 'select' does not support parameter of type 'name'.
    
      on /tmp/tf-test888880187/main.tf line 23:
      (source code not available)
    
    
    
    Error: 010716de:3: Policy '/Common/Drafts/http_to_https_redirect', rule 'http_to_https_redirect_rule'; target 'http-reply' action 'redirect' does not support parameter of type 'name'.
    
      on /tmp/tf-test888880187/main.tf line 39:
      (source code not available)

May be we should not convert all parameters to camel case, we should convert only specific parameters as defined in policy structure.

Can you check it once.

@thorhs
Copy link

thorhs commented Mar 16, 2020

For me, leaving out tm_name = "20" in the test policies, in bigip/resource_bigip_ltm_policy_test.go makes this work. Looking at the JSON that is being sent over to the F5 box, it seems that tm_name is being ignored in the master branch, but being converted to tmName in this new version.

What I suppose is happening is that there is no Tm_name in the F5 data structure, so it is being silently ignored. Below is an example of the failing JSON, when run using this PR:

{
    "controls": [
        "forwarding"
    ],
    "name": "Drafts/http_to_https_redirect",
    "publishedCopy": "",
    "requires": [
        "http"
    ],
    "rulesReference": {
        "items": [
            {
                "actionsReference": {
                    "items": [
                        {
                            "httpReply": true,
                            "location": "tcl:https://[HTTP::host][HTTP::uri]",
                            "name": "0",
                            "redirect": true,
                            "tmName": "20"
                        }
                    ]
                },
                "conditionsReference": {},
                "name": "http_to_https_redirect_rule",
                "ordinal": 0
            }
        ]
    },
    "strategy": "/Common/first-match"
}

And here is the JSON from the master branch version of the code:

{
    "controls": [
        "forwarding"
    ],
    "name": "Drafts/http_to_https_redirect",
    "publishedCopy": "",
    "requires": [
        "http"
    ],
    "rulesReference": {
        "items": [
            {
                "actionsReference": {
                    "items": [
                        {
                            "httpReply": true,
                            "location": "tcl:https://[HTTP::host][HTTP::uri]",
                            "name": "0",
                            "redirect": true
                        }
                    ]
                },
                "conditionsReference": {},
                "name": "http_to_https_redirect_rule",
                "ordinal": 0
            }
        ]
    },
    "strategy": "/Common/first-match"
}

As you can see, there is no tm_name being sent over the wire when using the master branch.

Removing the tm_name from the test case makes the tests run successfully again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants