Repository: ember-cli/ember-cli
Pull request #5986 (2016-06-15 13:58:33)

added transparent-proxy option to ember serve command
with respect to https://github.com/ember-cli/ember-cli/commit/4fc7ce26ef17881618420e0852a9bdfcaa53a4ce

Turbo87: how do you set this option to `false`?
badazz91: ```ember s --proxy http://localhost:3000 --transparent-proxy false```
Turbo87: hmm, wouldn't it be easier for the users if we used something like `--intransparent-proxy` without having to specify `true/false`?
badazz91: Yes, I agree. This would be better. Should I update the PR?
Turbo87: > Should I update the PR?

yes please :)
badazz91: I have updated the PR.

You can now enable the option like this:
``ember s --proxy http://localhost:3000 --intransparent-proxy`` or short style like this
``ember s --proxy http://localhost:3000 --intransp``.


nathanhammond: "intransparent" is such an awkward word (transparent's typical opposite is opaque, not negation). Let's wait a day and see if the hive mind can come up with a better name before landing?

Hey everybody! Naming help please!
badazz91: Hmm ```--opaque-proxy``` ...?

One could also think about ```--no-transparent-proxy``` or ```--non-transparent-proxy```?
Turbo87: `--no-x-fwd` because it's so much fun typing that :P
stefanpenner: looks good, but i do appreciate the further exploration of the short version of the flag.
badazz91: I like ```--no-x-fwd```, but nevertheless I also could stick with ```--intransparent-proxy```, because the ```x-forwared-headers-*```are used to make the requests ```transparent```, when proxying. According to wikipedia ... ;-)
stefanpenner: > I like --no-x-fwd, but nevertheless I also could stick with --intransparent-proxy,

both seems great IMHO
badazz91: Let me now, and I will update the PR ... again ;-)
badazz91: On the other hand, in my opinion, ```--intransparent-proxy```is a little bit more explicit than ```--no-x-fwd```, because it describes the actual behavior, and not what it actually does to achieve that. Furthermore I think it also aligns more with the option ```--insecure-proxy```. But I also understand, that ``opaque`` is better than ``intransparent``.

What's the general opinion on this?


stefanpenner: > --intransparent-proxy

lets do, individuals that use this commonly can set it in `.emberrc` file and it ends up being self documenting.


badazz91: FYI, there was already one failing test, when I forked from the master branch.
rwjblue: 👍 to the feature, but 👎 to the word `intransparent` 😝 

Perhaps, `--transparent-proxy` (along with the `--no-transparent-proxy` option allowed for boolean flags via `nopt`)?  If the flag of `transparent-proxy` is set to a boolean these two should work properly, and defaulting to `false` gets us the desired behavior (roughly what we have in this PR already).
badazz91: @rwjblue ```--transparent-proxy false``` is what the flag in the first version was, before I changed it to ```--intransparent-proxy``` in order to save some typing efffort like suggested by @Turbo87 .


rwjblue: @badazz91:

Concrete proposal: 

* change the flag to `transparent-proxy`
* change the default value to `true`
* change the `xfwd` option to just use the flag directly (no negation)

With this setup:

* The current defaults are preserved (using `xfwd` by default)
* Disabling is as easy as `--no-transparent-proxy` (which is only a single char longer than `--intransparent-proxy`)
* My brain doesn't hurt trying to grok what `--intransparent-proxy=true`, `--intransparent-proxy=false`, `--no-intransparent-proxy` all do...
Turbo87: can we make sure somehow that `--no-transparent-proxy` is included in the help text?
badazz91: @rwjblue @Turbo87 to clearify. I will add :

1. ```transparent-proxy false (default: true)```
2. ```--no-transparent-proxy``` and  really short ```--ntransproxy```

which both can be used to disable xfwd.
fpauser: I would not add the `--no-transparent-proxy` because as a consequence of this all other boolean-options should also have their `--no-*` counterparts which leads to confusion due to many (possibly unclear/conficting) options.

I would vote for just adding the `--transparent-proxy` option that defaults to `true`.

Maybe it would be a good idea to also add a `--secure-proxy` option that defaults to true and to deprecate the `--insecure-proxy` option over the time, because `--insecure-proxy` right now is the only negating-option - and somehow not in-line with the other boolean-options.
badazz91: After further consideration, I think it is the best to leave it at just  ```--transparent-proxy false```. It is the most explicit way, in my opinion. As suggested by @stefanpenner, people who use it more often can config it in their ```.emberrc``` to save some typing effort. Furthermore I also agree with @fpauser's idea, to rename ```--insecure-proxy``` to ```--secure-proxy``` for better alignment with the other options.


nathanhammond: Okay, so it sounds to me like the proposal here is "no further change to this PR as it stands."

I kicked over the failure on Travis without investigation. Assuming that passes, this is last call for comments before merge.
badazz91: Strangely, Travis still has a failing test using `node v0.12.4`, that I cannot reproduce locally. All tests pass...

> 
  909 passing (18s)
  5 pending

> 
Mocha Tests Running Time: 20056ms

stefanpenner: @badazz91 rerunning the node 0.12 test
badazz91: @stefanpenner It still seems to fail. I investigated the test again, and also reran it with ```node v0.12.14```. Looks both good to me ... Why does it still fail? :(

Patch:
diff --git a/lib/commands/serve.js b/lib/commands/serve.js
index 336487409..a91146963 100644
--- a/lib/commands/serve.js
+++ b/lib/commands/serve.js
@@ -23,6 +23,7 @@ module.exports = Command.extend({
     { name: 'host',                 type: String,                          aliases: ['H'],     description: 'Listens on all interfaces by default' },
     { name: 'proxy',                type: String,                          aliases: ['pr', 'pxy'] },
     { name: 'insecure-proxy',       type: Boolean, default: false,         aliases: ['inspr'], description: 'Set false to proxy self-signed SSL certificates' },
+    { name: 'transparent-proxy',    type: Boolean, default: true,          aliases: ['transp'], description: 'Set to false to omit x-forwarded-* headers when proxying' },
     { name: 'watcher',              type: String,  default: 'events',      aliases: ['w'] },
     { name: 'live-reload',          type: Boolean, default: true,          aliases: ['lr'] },
     { name: 'live-reload-host',     type: String,                          aliases: ['lrh'],   description: 'Defaults to host' },
diff --git a/lib/tasks/server/middleware/proxy-server/index.js b/lib/tasks/server/middleware/proxy-server/index.js
index 6aded187f..a4e21fe84 100644
--- a/lib/tasks/server/middleware/proxy-server/index.js
+++ b/lib/tasks/server/middleware/proxy-server/index.js
@@ -15,7 +15,7 @@ ProxyServerAddon.prototype.serverMiddleware = function(options) {
       ws: true,
       secure: !options.insecureProxy,
       changeOrigin: true,
-      xfwd: true
+      xfwd: options.transparentProxy
     });
 
     proxy.on('error', function (e) {
diff --git a/package.json b/package.json
index 95e4045b3..3c53f70e8 100644
--- a/package.json
+++ b/package.json
@@ -61,7 +61,7 @@
     "core-object": "^2.0.2",
     "debug": "^2.1.3",
     "diff": "^2.2.2",
-    "ember-cli-broccoli": "0.16.10",
+    "ember-cli-broccoli": "0.16.9",
     "ember-cli-get-component-path-option": "^1.0.0",
     "ember-cli-is-package-missing": "^1.0.0",
     "ember-cli-legacy-blueprints": "^0.1.1",


Test patch:
diff --git a/tests/fixtures/help/help-with-addon.txt b/tests/fixtures/help/help-with-addon.txt
index 9b57457b2..15c0ad5ba 100644
--- a/tests/fixtures/help/help-with-addon.txt
+++ b/tests/fixtures/help/help-with-addon.txt
@@ -122,6 +122,8 @@ ember serve \u001b[36m<options...>\u001b[39m
     \u001b[90maliases: -pr <value>, -pxy <value>\u001b[39m
   \u001b[36m--insecure-proxy\u001b[39m \u001b[36m(Boolean)\u001b[39m \u001b[36m(Default: false)\u001b[39m Set false to proxy self-signed SSL certificates
     \u001b[90maliases: --inspr\u001b[39m
+  \u001b[36m--transparent-proxy\u001b[39m \u001b[36m(Boolean)\u001b[39m \u001b[36m(Default: true)\u001b[39m Set to false to omit x-forwarded-* headers when proxying
+    \u001b[90maliases: --transp\u001b[39m
   \u001b[36m--watcher\u001b[39m \u001b[36m(String)\u001b[39m \u001b[36m(Default: events)\u001b[39m
     \u001b[90maliases: -w <value>\u001b[39m
   \u001b[36m--live-reload\u001b[39m \u001b[36m(Boolean)\u001b[39m \u001b[36m(Default: true)\u001b[39m
diff --git a/tests/fixtures/help/help.js b/tests/fixtures/help/help.js
index ace25366c..128f78073 100644
--- a/tests/fixtures/help/help.js
+++ b/tests/fixtures/help/help.js
@@ -480,6 +480,14 @@ module.exports = {
           key: 'insecureProxy',
           required: false
         },
+        {
+          name: 'transparent-proxy',
+          default: true,
+          description: 'Set to false to omit x-forwarded-* headers when proxying',
+          aliases: ['transp'],
+          key: 'transparentProxy',
+          required: false
+        },
         {
           name: 'watcher',
           default: 'events',
diff --git a/tests/fixtures/help/help.txt b/tests/fixtures/help/help.txt
index a67e3c841..743cd2a0c 100644
--- a/tests/fixtures/help/help.txt
+++ b/tests/fixtures/help/help.txt
@@ -122,6 +122,8 @@ ember serve \u001b[36m<options...>\u001b[39m
     \u001b[90maliases: -pr <value>, -pxy <value>\u001b[39m
   \u001b[36m--insecure-proxy\u001b[39m \u001b[36m(Boolean)\u001b[39m \u001b[36m(Default: false)\u001b[39m Set false to proxy self-signed SSL certificates
     \u001b[90maliases: --inspr\u001b[39m
+  \u001b[36m--transparent-proxy\u001b[39m \u001b[36m(Boolean)\u001b[39m \u001b[36m(Default: true)\u001b[39m Set to false to omit x-forwarded-* headers when proxying
+    \u001b[90maliases: --transp\u001b[39m
   \u001b[36m--watcher\u001b[39m \u001b[36m(String)\u001b[39m \u001b[36m(Default: events)\u001b[39m
     \u001b[90maliases: -w <value>\u001b[39m
   \u001b[36m--live-reload\u001b[39m \u001b[36m(Boolean)\u001b[39m \u001b[36m(Default: true)\u001b[39m
diff --git a/tests/fixtures/help/with-addon-blueprints.js b/tests/fixtures/help/with-addon-blueprints.js
index 18942a8cb..322feb362 100644
--- a/tests/fixtures/help/with-addon-blueprints.js
+++ b/tests/fixtures/help/with-addon-blueprints.js
@@ -512,6 +512,14 @@ module.exports = {
           key: 'insecureProxy',
           required: false
         },
+        {
+          name: 'transparent-proxy',
+          default: true,
+          description: 'Set to false to omit x-forwarded-* headers when proxying',
+          aliases: ['transp'],
+          key: 'transparentProxy',
+          required: false
+        },
         {
           name: 'watcher',
           default: 'events',
diff --git a/tests/fixtures/help/with-addon-commands.js b/tests/fixtures/help/with-addon-commands.js
index 01bdca53d..eddc55ae0 100644
--- a/tests/fixtures/help/with-addon-commands.js
+++ b/tests/fixtures/help/with-addon-commands.js
@@ -480,6 +480,14 @@ module.exports = {
           key: 'insecureProxy',
           required: false
         },
+        {
+          name: 'transparent-proxy',
+          default: true,
+          description: 'Set to false to omit x-forwarded-* headers when proxying',
+          aliases: ['transp'],
+          key: 'transparentProxy',
+          required: false
+        },
         {
           name: 'watcher',
           default: 'events',
diff --git a/tests/unit/commands/serve-test.js b/tests/unit/commands/serve-test.js
index a7273e7cb..66102a9d7 100644
--- a/tests/unit/commands/serve-test.js
+++ b/tests/unit/commands/serve-test.js
@@ -183,6 +183,14 @@ describe('serve command', function() {
     });
   });
 
+  it('has correct default value for transparent proxy', function() {
+    return command.validateAndRun().then(function() {
+      var captor = td.matchers.captor();
+      td.verify(tasks.Serve.prototype.run(captor.capture()), {times: 1});
+      expect(captor.value.transparentProxy).to.equal(true, 'has correct transparent proxy option when not set');
+    });
+  });
+
   it('uses baseURL of correct environment', function() {
     options.project.config = function(env) {
       return { baseURL: env };