Skip to content
This repository has been archived by the owner on May 10, 2018. It is now read-only.

Secure "env" configuration - continuation #76

Merged
merged 11 commits into from Jul 9, 2012

Conversation

drogus
Copy link
Contributor

@drogus drogus commented Jul 4, 2012

I rebased the work that @laserlemon pushed in his pull request (#45) and ensured that ENV variables are not decrypted for pull requests.

I haven't added TRAVIS_SECURE_ENV_VARS and TRAVIS_PULL_REQUEST env vars yet, as I'm not sure what is the best place to do it. For now I added pull_request key to the API and I planned to export those variables in the worker, but I'm not familiar with travis' architecure yet, so I would like to get some feedback for this.

The other thing is that I don't like the fact that we need all or nothing approach. If you want to have secure ENV vars, you need to encrypt all of them and because of that there is no way to set both types of vars. Can I also address it in this pull request? And if yes, have you guys thought about approach to this problem or is this something that hasn't been discussed before? (disregard this paragraph, travis is right, I'm wrong)

@travisbot
Copy link

This pull request passes (merged f4deb969 into 8afd339).

@drogus
Copy link
Contributor Author

drogus commented Jul 4, 2012

@joshk I've updated this PR with fix to allow handling arrays for env vars, like:

- env:
  - ["FOO=bar", {secure: "encrypted..."}]

I'm pretty sure that this should be already handled in worker, according to export method in travis-build: https://github.com/travis-ci/travis-build/blob/master/lib/travis/build/job/test.rb#L106-110, but please let me know if there are other places that I should look at.

@drogus
Copy link
Contributor Author

drogus commented Jul 4, 2012

Also, implementation of decrypting and building matrix is getting kind of messy, so I will look into refactoring it, but I just wanted to send this quick spike, to show the fix and get feedback.

@travisbot
Copy link

This pull request passes (merged 449a6c79 into 8afd339).

@travisbot
Copy link

This pull request passes (merged 3dfd3bd7 into 0db5530).

@joshk
Copy link
Contributor

joshk commented Jul 4, 2012

This looks good to me, although i think we need to merge in the changes to travis build first, including the TRAVIS_ stuff, wdyt?

@drogus
Copy link
Contributor Author

drogus commented Jul 5, 2012

@joshk sure! I didn't have time to finish it and push, I will do that soon. For now, I refactored matrix_config by extracting it to separate class, I have left most of the methods unchanged, but this opens way for more changes and (what's probably even better for me) to change the tests - I don't like the fact that I need to setup entire Build to test if config is correctly prepared. Other than that I think that's ready to go :)

@travisbot
Copy link

This pull request passes (merged 43fd07f5 into 0db5530).

@drogus
Copy link
Contributor Author

drogus commented Jul 5, 2012

@joshk I pushed changes in travis-ci/travis-build#22, travis-ci/travis-worker#31 also will need to be merged (this one is from @laserlemon)

laserlemon and others added 11 commits July 7, 2012 22:38
If you want to encrypt some of the ENV variables, but leave the others
untouched, you need to pass array in config, like:

    - env:
      - ["FOO=bar", {secure: "encrypted..."}]

Worker can handle such payload, but decrypting and sending such payload was
broken. This commit fixed this situation to allow defining arrays like the one
above.
This is just a start of refactoring matrix related methods. I moved all of those
methods to separate class (Matrix::Config) to clean Matrix module.
It can be used instead of config when we want to display config with encrypted
ENV variables in human readable way. For example instead of displaying:

  { :env => [{:secure => "abf8a7bd8e0......."}] }

you may display:

  { :env => ["FOO=[secure] BAR=[secure]"] }
@travisbot
Copy link

This pull request fails (merged 6cf1e27 into f7fc748).

@drogus
Copy link
Contributor Author

drogus commented Jul 7, 2012

@svenfuchs could you take a look on 7b3fe68 and 6cf1e27? @joshk noticed that it will probably look bad to display encrypted vars in the matrix on site, so I added obfuscation to config in the API

@svenfuchs
Copy link
Contributor

@drogus that makes sense to me

drogus added a commit that referenced this pull request Jul 9, 2012
Secure "env" configuration - continuation
@drogus drogus merged commit a7097c6 into travis-ci:master Jul 9, 2012
@pangratz
Copy link

Is this already usable or is this not yet available on travis-ci.org?

@joshk
Copy link
Contributor

joshk commented Jul 17, 2012

Not yet available, there will be a blog post when it is.

On 17/07/2012, at 3:27 PM, Clemens Müller wrote:

Is this already usable or is this not yet available on travis-ci.org?


Reply to this email directly or view it on GitHub:
#76 (comment)

@pangratz
Copy link

Great! Thanks for the immediate feedback!

@cmanzana
Copy link

I have made a 'node figaro' module (https://github.com/cmanzana/node-figaro) to take this feature into use in node.js applications
As soon as the feature is available in travis I will test it with node publish (https://github.com/cmanzana/node-publish)

@pangratz
Copy link

@cmanzana Nice!! Looks great!

@laserlemon
Copy link
Contributor

@svenfuchs @joshk @drogus If not live yet, is there anything I can do to help out? Thanks guys! This looks great. 👏

@joshk
Copy link
Contributor

joshk commented Oct 2, 2012

This is all deployed and we should have docs on it too :)

Thank you sooooo much for helping get this live!

Do try it out and let us know how it gets on for you!

On 2/10/2012, at 1:36 PM, Steve Richert wrote:

@svenfuchs @joshk @drogus If not live yet, is there anything I can do to help out? Thanks guys! This looks great.


Reply to this email directly or view it on GitHub.

@laserlemon
Copy link
Contributor

Fantastic. Will do. 🤘

@farmdawgnation
Copy link

@joshk I can confirm it's working, but I had to figure that out myself. I haven't seen any docs for it on the Travis site...

@laserlemon
Copy link
Contributor

@laserlemon
Copy link
Contributor

I also submitted a pull request to fix Travis encryption through the CLI gem: https://github.com/travis-ci/travis-cli/pull/4

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
8 participants