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

Default to port 443 if UseSSL set. #234

Merged
merged 3 commits into from
Mar 25, 2021

Conversation

Integralist
Copy link
Collaborator

Problem: if --use-ssl flag is provided but no port is set, then we should default to port 443 and also warn the user about this behaviour if the --verbose flag is provided.

Notes: you can't set a port number as part of the address field (e.g. 127.0.0.1:443) as the API considers that invalid...

$ curl -s -H "Accept:application/vnd.api+json" -H "Fastly-Key: $FASTLY_API_KEY" -X PUT -d "address=127.0.0.1:443" https://api.fastly.com/service/4nE2aoohVrsds0OZGpSIP8/version/1/backend/httpbin | jq

{              
  "errors": [   
    {                     
      "detail": "Address 'address' is not a valid IPv4, IPv6 or hostname",
      "title": "Bad request"
    }                                                                                                                                                                                                                                         
  ]            
}   

@Integralist Integralist requested review from acme, phamann, a team and doramatadora and removed request for a team March 24, 2021 15:51
@Integralist Integralist added the enhancement New feature or request label Mar 24, 2021
Copy link
Member

@phamann phamann left a comment

Choose a reason for hiding this comment

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

Looks good to me other than minor nit around terminology in the verbose output. It would have been good to also have a test to actually assert 443 was set if no port was supplied, however that won't be possible until we refactor these command tests to mirror the patterns used in the logging endpoints.

pkg/backend/create.go Outdated Show resolved Hide resolved
@Integralist Integralist force-pushed the integralist/20210324_default_443_with_usessl branch from 2d54489 to 9648dd4 Compare March 25, 2021 11:45
@Integralist Integralist merged commit 076b769 into master Mar 25, 2021
@Integralist Integralist deleted the integralist/20210324_default_443_with_usessl branch March 25, 2021 12:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants