Skip to content
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.

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

[rush] SOC compliance; Any file change to code that's to be to the main branch MUST to be reviewed and approved by at least one other person #3012

Open
renoirb opened this issue Nov 5, 2021 · 9 comments

Comments

@renoirb
Copy link
Contributor

renoirb commented Nov 5, 2021

Find ways to allow use of Rush and respect corporate decisions that involves security compliance.

The requirement this issue revolves around is:

ANY file change that's to be written to the main branch MUST to be reviewed and approved by at least one other person

This situation makes it hard to convince company's security team to let teams to use Rush.

Even to a point where it is accepted to deal with a DIY build setup and scripts merely partially supporting a fraction of what Rush does for free.

Rush isn't the only tool to manage monorepos.

Requirements

MUST be reviewed and approved by at least one other person

In organizations that has security audit and policies to follow for code changes, one of the decisions may involve forcing ANY type of change to code that's committed to source control to be reviewed and approved by at least one other person.

The requirement might be as broad as requiring an approval even for package.json version change, and CHANGELOG.json text insertions.

When the line is drawn at commit, what teams do to support releasing multiple packages is to:

  • Keep package.json to a static string that never gets changed
  • Edit package.json during build to have new version just before npm pack, and npm publish
  • Any other changes inside the package

What would be great is:

  • Add changelog data for each package' inside git tag
    • Assuming we could optionally not bump the package.json version
    • Having the changelog written in the git tag would remove the need to "approve" a commit for changelog text
  • OR, Allow usage of an HTTP "publishing" adapter where we could instead do the git tag using that API instead of from

Related Zulip conversations:

Any merge to main and allow trigger build steps

To trigger a publish and deploy, it would be awesome to be able to know if a current changeset involves a "publishable" package. (probably using --to package1 --to package2 select subsets of projects.

So we could know which packages were impacted during a release, if a changeset only involve shouldPublish: false packages, the team would do something different than when to publish.

Related issues:

Out of scope

In the same type of requests for the sake of compliance the organization and operations team might be:

  • Enforcing git commit messages to match some linting rules and would be part of the changelog. (Semantic Commit) and using that for resolving how to bump the next release

Summary

Repro steps

Expected result:

Actual result:

Details

Standard questions

Please answer these questions to help us investigate your issue more quickly:

Question Answer
@microsoft/rush globally installed version?
rushVersion from rush.json?
useWorkspaces from rush.json?
Operating system?
Would you consider contributing a PR?
Node.js version (node -v)?
@renoirb
Copy link
Contributor Author

renoirb commented Nov 5, 2021

Hi @pgonzal here's what I could write. Can you help me better word it?

@renoirb renoirb changed the title [rush] [rush] SOC compliance; Any file change to code that's to be to the main branch MUST to be reviewed and approved by at least one other person Nov 5, 2021
@wbern
Copy link
Contributor

wbern commented Nov 6, 2021

I don't quite understand this, are you implying that companies using rush should be added as required reviewers before merging code into master?

@renoirb
Copy link
Contributor Author

renoirb commented Nov 6, 2021

Rush is a tool to maintain many packages and one of the target audience are corporations and their teams. Software development teams may have to follow directives from management. Corporations that makes products that involve other people's private data might have to follow data privacy laws. At least one of them require that there is a peer review (and track record) for every line of code running in production.

Interpretation of how those policies are enforced can be debated and implemented different in each organization.

If one organization follow to the letter that ANY changes that goes on the main branch, including simplest version number increment in a package.json, and CHANGELOG text content. That we know isn't code. The rationale for not picking Rush because of that can be above the programmer, who wants to bring in Rush, paygrade.

That would prevent a team to use this fine project that I personally cherish.

Which is my situation. Been hired because of, in part because of experience with large number of packages in monorepo, and can't use the tool I know and work best with because of politics.

@wbern
Copy link
Contributor

wbern commented Nov 6, 2021

Isn't this kind of a broken argument, seeing as rush is version controlled and you are free to review the changes before bumping your dependency?

@renoirb
Copy link
Contributor Author

renoirb commented Nov 6, 2021

When there's a release the rush publish writes those things and merges to master, with changelog and version bump and tag.

That part. When in continuous release where each merge create some "releasing" and publishing and deploying. The file writes, like version bump, in large organization where you're only a team of a handful of people, forbids using the tool. Because "it won't work in our process".

Even though SOC compliance would agree that a text document describing changes, and a number bump acceptable.

@octogonz said maybe we could find the standardization in security body that publish SOC to say, in that case, would ot cause issue. Because it's just number bump and a changelog.

So, we can see how to make Rush publish using an HTTP adapter to a Git hosting (GitLab, GitHub, BitBucket, to their git tag API endpoing).

Or seek to see a proper public statement.

Helping teams who wants to use Rush but be forbidden by this.

@wbern
Copy link
Contributor

wbern commented Nov 6, 2021

Ok, so you mean that packages published inside your own rush repo need a workflow that works for your needs, not the rush tool itself?

@renoirb
Copy link
Contributor Author

renoirb commented Nov 6, 2021

Actually, I've created this ticket by request of @octogonz during last Rush-Hour because what I'm describing (admittedly clumsily) was recognized as something to track.

The description needs work.

A proposed path would be as part of the current publish workflow redesign. And/or way to do the publish last mile using a different strategy.

For example, I've heard Microsoft's teams using rush itself internally for products doesn't use the git tag from rush publish but rather an API call.

I can see how to do it, I have stashed code doing this, and rush-lib ChangeAnalyzer to help.

This issue is describing a usage context and use-case for the current rush-publish-requirements gathering:

@wbern
Copy link
Contributor

wbern commented Nov 7, 2021

I understand. 🙂 Yes it was a little confusing at first.

@elliot-nelson
Copy link
Collaborator

I'd like to provide an update on this concept. In the last Rush Hour West I gave an overview of https://github.com/changesets/changesets, which is a stand-alone tool that offers a version of Rush's "change files" functionality.

A notable difference is that Changesets' current approach for publishing is to create a PR which consumes change files and updates change logs / version bumps; this PR is essentially "ready" to be reviewed and merged by a maintainer, and that's the act that triggers publishing the packages. This model falls in line with the desire in the OP (no machine-generated commits to trunk without a human approval.)

See tracking ticket #3835 for an overview of work to get Changesets compatible with Rush, and possibly to bring the two models closer together so either the "direct commit to trunk" or "PR publish" models are available in Rush.

@iclanton iclanton moved this to General Discussions in Bug Triage Aug 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: General Discussions
Development

No branches or pull requests

3 participants