cs-education / sysassets

Larger precompiled assets for the sys/ project
Apache License 2.0
1 stars 10 forks source link

Added xdomain functionality for sysbuild #4

Closed scowalt closed 9 years ago

scowalt commented 9 years ago

Read this issue in the sysbuild project for more background on this pull request. Some things I'm unsure of:

  1. Should we pull the javascript library from the rawgit cdn or include the file locally in the repository?
  2. Should we include localhost as one of the masters? This allows us test on our local machines much easier, but may open us up to a security or other vulnerability I'm not aware of. If we're going to allow localhost anyways, is it equally secure to just allow all domains?
wchill commented 9 years ago
  1. Perhaps use a Git submodule to ensure you get a known working version without having to bundle a copy of the library directly (which is just clunky and makes updating said library a hassle). Refer to http://git-scm.com/book/en/v2/Git-Tools-Submodules
  2. As far as I'm aware, localhost is only an issue if the user's machine is already compromised, as any malicious code that might be an issue must already reside on the user's computer. However, allowing all domains creates potential for XSS attacks in ways that localhost does not, as attackers can use a domain/host that is already under their control.
scowalt commented 9 years ago

Hey Eric, thanks for the feedback!

Would using submodules be more cumbersome for developers setting up the project? It seems like this turns git clone <url> into: (source)

git clone <url>
git submodule init
git submodule update

Even if this is the case, using submodules may be the best solution. And it's confirmed to work with Github Pages automatically.

wchill commented 9 years ago

This can be replaced with one command: (source)

git clone <url> --recursive

To be fair, I am not all that familiar with using submodules, but in either case, it seems like it would be a one-time thing, and details on this process can be added to the README.

scowalt commented 9 years ago

Alright, I included the library as a git submodule and confirmed that git clone <url> --recursive will take care of getting the dependency. I think this pull request is good to go.

angrave commented 9 years ago

On 2/18/15 3:46 PM, Scott Walters wrote:

Alright, I included the library as a git submodule and confirmed that |git clone --recursive| will take care of getting the dependency. I think this pull request is good to go.

— Reply to this email directly or view it on GitHub https://github.com/angrave/sysassets/pull/4#issuecomment-74954872.

The production and staging build scripts should verify that submodules are at the correct version number.

scowalt commented 9 years ago

Lawrence,

Git submodules stay locked at a specific commit unless directed to update, so xdomain's version number won't change. (source) Do you mean checking the xdomain version number in sysassets during the build/deploy process of sysbuild? Our plan is not to include sysassets inside of sysbuild, but rather to use sysassets as an external resource.

angrave commented 9 years ago

(I'm not going to answer your q directly. Instead here's the spirit of the idea)

I mean that the sys deploy should be independent of who is doing it. It is possible to have a local git submodules at different version from somebody else; hence the concern for reliable builds

i.e. deploys must be reliable and repeatable and not dependent on local state e.g. if you changed your local submodule state (e.g. by updating from the remote), then it should not be possible to deploy to staging( without a warning) or production (=error).

So ideally: the build process would ensure that all files are committed and the submodules version are in a known/well-defined state, before pushing to production.

L.

On 2/19/15 3:49 PM, Scott Walters wrote:

Lawrence,

Git submodules stay locked at a specific commit unless directed to update, so xdomain's version number won't change. (source http://git-scm.com/docs/git-submodule) Do you mean checking the xdomain version number in sysassets during the build/deploy process of sysbuild? Our plan is not to include sysassets inside of sysbuild, but rather to use sysassets as an external resource.

— Reply to this email directly or view it on GitHub https://github.com/angrave/sysassets/pull/4#issuecomment-75145316.

neelabhg commented 9 years ago

@angrave, there is no build process for this project (there is, but not for JS code). As such, this is not like sysbuild in which we have a build step and then a publish step. The contents of this repository are exactly what are published (this project has gh-pages as the default branch).

So, even if a developer makes changes to the submodule code, they will not be pushed here because git only tracks the submodule version, not the contents itself. The developer must change the commit version of the submodule, in which case the change can be reviewed in a pull-request.

In fact, your concern is valid for sysbuild, where a person deploying can change versions of dependencies before publishing, in which case the published site and the development project will have different versions for dependencies. However, this will not happen if using the build tools and dependency management tools properly. Someone will have to go out of their way to create a mismatch in dependency versions.

So, I think, deployments of this repository are safe (in fact, as mentioned above, there is no "deployment" for this project - the source code is the deployed website).

neelabhg commented 9 years ago

@scowalt Why are we using a submodule instead of simply using some dependency management system like bower, or directly using the library from a CDN? As far as I understand after reading the documentation on Git Submodules, submodules are good when we need to modify the source code of the tracked project as well - but this is not the case with us. In fact, the link above mentions this:

It’s quite likely that if you’re using submodules, you’re doing so because you really want to work on the code in the submodule at the same time as you’re working on the code in the main project (or across several submodules). Otherwise you would probably instead be using a simpler dependency management system (such as Maven or Rubygems).

In any case, I think we don't need to change anything here, as submodules work well with GitHub pages and you should not waste your time on moving away from using a submodule.

scowalt commented 9 years ago

@neelabhg, I'm using a submodule to avoid having to check the library itself into version control. This also allows us to avoid being reliant on an external CDN or adding an additional build step to the deployment of sysassets

neelabhg commented 9 years ago

Oh I see. That makes a lot of sense - because the source code is the deployed project, one needs to include the library in the source which is less than ideal. Good decision.

neelabhg commented 9 years ago

We are merging this in so that we can continue work on dependent issue cs-education/sysbuild#70.