Strider-CD / strider-github

Github provider for strider
25 stars 34 forks source link

Not registering the Environment variables #32

Closed rayman22201 closed 8 years ago

rayman22201 commented 9 years ago

I set the environment variables as per the documentation, but Strider seems to be ignoring them.

I used the following environment variables: PLUGIN_GITHUB_APP_ID PLUGIN_GITHUB_APP_SECRET PLUGIN_GITHUB_HOSTNAME

I don't see any references to those variables in the code. Is the Strider extension loader is supposed to auto-load environment variables into the config object? :confounded:

For now, I made a fork with the following patch (read hack) that seems to work. It explicitly looks for those environment vars in the config object declaration: https://github.com/Galavantier/strider-github/blob/master/lib/webapp.js#L11-L13

I found this original issue that solved the problem, but it doesn't seem to be working now: https://github.com/Strider-CD/strider-github/issues/22

Any ideas about what is going on?

My patch seems to be working, but I would really prefer to help get the official repo working instead of maintaining a fork :smile:

Thanks so much

knownasilya commented 9 years ago

Can you try master.

rayman22201 commented 9 years ago

I did a pull from master. It does not work. :-(

If it helps. I am using a docker container that is pulling the master branch of Strider itself, as well as pulling master from this repo.

-------------------- update --------------------- I put some debug statements in the code to see if I could garner any more info.

It looks like the APP ID and APP SECRET are correctly set, but the hostname is not. It is still set to localhost.

knownasilya commented 9 years ago

Do you have SERVER_NAME set, as per the instructions here?

rayman22201 commented 9 years ago

Ah ha! I set the SERVER_NAME environment variable, and that worked. :+1: :smile:

is PLUGIN_GITHUB_HOSTNAME used at all?

Thanks so much for your help.

knownasilya commented 9 years ago

Maybe not.. I'll look through the code when I get a chance, and if it's not we should remove it from the README.

trein commented 9 years ago

Hi @knownasilya and @rayman22201,

I'm facing a similar problem here. I'm using the plugin version 1.3.2, but I'm still getting localhost (http://localhost:3000/auth/github/callback) when adding a new account on account settings.

Here is my upstart script:

start on mounted MOUNTPOINT=/
stop on shutdown

env STRIDER_HOME=/opt/strider
env SERVER_NAME=http://myurl.com:1234
env PORT=1234
env DB_URI=mongodb://localhost:27017/strider-foss

script
    chdir $STRIDER_HOME
    exec npm start 1>>/opt/strider/log/stdout.log 2>>/opt/strider/log/stderr.log
    emit strider_running
end script

Thanks,

knownasilya commented 9 years ago

@trein try removing the port from SERVER_NAME.

trein commented 9 years ago

Thanks for the reply @knownasilya. I removed the port, but the issue persists.

knownasilya commented 9 years ago

@trein not sure, but your example has a typo: http:/myurl.., there is only one slash.

trein commented 9 years ago

@knownasilya sorry about that. I had to mask the real URL. It's just a typo on the comment. I'm going to fix it. On /etc/init/strider.conf it is correct.

knownasilya commented 9 years ago

Are any of the variables passing to strider, like the port? If you can confirm that, we can try other things.

trein commented 9 years ago

@knownasilya No. That's all that is being passed.

knownasilya commented 9 years ago

What I meant, is that in your example you show setting PORT, can you confirm that Strider is running on that port (via netstat or something)?

If you aren't setting it, could you, just as a test.

trein commented 9 years ago

@knownasilya Yes, it is running on port 1234.

knownasilya commented 9 years ago

And you set both: PLUGIN_GITHUB_APP_ID and PLUGIN_GITHUB_APP_SECRET?

Also if you tried to auth with github before setting the SERVER_NAME, there might be an entry in github that references the localhost callback url. See if there is, and delete it.

trein commented 9 years ago

Hi @knownasilya. That was a silly mistake. That's what I was missing! PLUGIN_GITHUB_APP_ID and PLUGIN_GITHUB_APP_SECRET weren't configured properly. I took over this task from another dev and haven't realized that he had forgotten to set them.

I apologize for the mistake. Thank you very much for the support.

knownasilya commented 9 years ago

No problem. Looks like we need to throw an error if SERVER_NAME is set and the other's aren't and you try to auth..

trein commented 9 years ago

Indeed. That would improve error tracing.

knownasilya commented 9 years ago

Hostname discrepancy resolved with #35

knownasilya commented 9 years ago

Opened #36 for a better error if needed variables aren't set

macropin commented 9 years ago

I had this same problem. In my case PORT=3000 wasn't set. Even though Strider is running on the default port (3000) and behind a load balancer (port 80) this plugin requires this config to be set. I would consider this to be a defect, as the default should be used.

knownasilya commented 9 years ago

Good to know.