Skip to content

Setting DisableCompression to true to disable unwanted httpclient transformation #4165

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
Merged
merged 1 commit into from
Jun 11, 2018
Merged

Setting DisableCompression to true to disable unwanted httpclient transformation #4165

merged 1 commit into from
Jun 11, 2018

Conversation

tariq1890
Copy link
Contributor

replacing PR #4164 and fixing issue #2916

@k8s-ci-robot
Copy link

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@k8s-ci-robot k8s-ci-robot added cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jun 3, 2018
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Jun 3, 2018
@tariq1890
Copy link
Contributor Author

@bacongobbler This PR complies with the CLA regulations. Let me know your review comments :)

@bacongobbler
Copy link
Member

This looks great. It's the weekend right now but give me some time to give this a spin next week just to be sure we don't accidentally break anything. Thanks so much for taking the time to fix this!

@tariq1890
Copy link
Contributor Author

Sure. Glad to help :)

@tariq1890
Copy link
Contributor Author

Hey @bacongobbler any luck testing this :) ?

@bacongobbler
Copy link
Member

Hey sorry, I've been away traveling all week. I'll have a look this morning. Thanks!

bacongobbler
bacongobbler approved these changes Jun 11, 2018
View reviewed changes
Copy link
Member

@bacongobbler bacongobbler left a comment

Choose a reason for hiding this comment

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

Manually tested this by pushing stable/traefik to a bitbucket repository, verifying we were getting back the wrong header response, then performed a helm fetch with this patch. The resultant tarball was fetched down as a gzipped tarball. LGTM!

><> ./bin/helm fetch https://bitbucket.org/bacongobbler/test-helm-charts/downloads/traefik-1.28.1.tgz
><> ./bin/helm install traefik-1.28.1.tgz
NAME:   enervated-gorilla
LAST DEPLOYED: Mon Jun 11 15:23:18 2018
NAMESPACE: default
STATUS: DEPLOYED

RESOURCES:
==> v1/ConfigMap
NAME                       DATA  AGE
enervated-gorilla-traefik  1     0s

==> v1/Service
NAME                       TYPE          CLUSTER-IP    EXTERNAL-IP  PORT(S)                     AGE
enervated-gorilla-traefik  LoadBalancer  10.98.216.74  <pending>    80:32114/TCP,443:30416/TCP  0s

==> v1beta1/Deployment
NAME                       DESIRED  CURRENT  UP-TO-DATE  AVAILABLE  AGE
enervated-gorilla-traefik  1        0        0           0          0s

==> v1/Pod(related)
NAME                                        READY  STATUS             RESTARTS  AGE
enervated-gorilla-traefik-64cbfc8d77-kzgdq  0/1    ContainerCreating  0         0s


NOTES:

1. Get Traefik's load balancer IP/hostname:

     NOTE: It may take a few minutes for this to become available.

     You can watch the status by running:

         $ kubectl get svc enervated-gorilla-traefik --namespace default -w

     Once 'EXTERNAL-IP' is no longer '<pending>':

         $ kubectl describe svc enervated-gorilla-traefik --namespace default | grep Ingress | awk '{print $3}'

2. Configure DNS records corresponding to Kubernetes ingress resources to point to the load balancer IP/hostname found in step 1

><> file --mime-type -b traefik-1.28.1.tgz
application/x-gzip

@bacongobbler
Copy link
Member

bacongobbler commented Jun 11, 2018 •
edited
Loading

Oh, and just to verify:

><> curl -Li https://bitbucket.org/bacongobbler/test-helm-charts/downloads/traefik-1.28.1.tgz
...

HTTP/1.1 200 OK
x-amz-id-2: YU3T6Z8MwzzCL3T0WMMyJannl4A16beUiLiX3JEj5CXzFXEDwK3eTc4PeOJ1IvMsK/iQYmFNymA=
x-amz-request-id: 7B85773293A8FC6B
Date: Mon, 11 Jun 2018 22:27:14 GMT
Last-Modified: Mon, 11 Jun 2018 22:20:53 GMT
ETag: "a928c6d6bde8d96deb333973a10e614c"
x-amz-version-id: HItgPm2INR0436OmfvQ0XlrKX66aMtKz
Content-Disposition: attachment; filename="traefik-1.28.1.tgz"
Accept-Ranges: bytes
Content-Type: application/x-tar
Content-Length: 15254
Server: AmazonS3

Warning: Binary output can mess up your terminal. Use "--output -" to tell
Warning: curl to output it to your terminal anyway, or consider "--output
Warning: <FILE>" to save to a file.

@bacongobbler bacongobbler merged commit ad38044 into helm:master Jun 11, 2018
@bacongobbler bacongobbler mentioned this pull request Jun 11, 2018
Closed
@Idane Idane mentioned this pull request May 7, 2020
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bacongobbler bacongobbler bacongobbler approved these changes

Assignees
No one assigned
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tariq1890 @k8s-ci-robot @bacongobbler