Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

[CI] allow cargo audit to fail #10676

Merged
merged 3 commits into from
May 20, 2019
Merged

[CI] allow cargo audit to fail #10676

merged 3 commits into from
May 20, 2019

Conversation

ordian
Copy link
Collaborator

@ordian ordian commented May 19, 2019 •
edited
Loading

cargo audit is something that is orthogonal to a PR and shouldn't prevent it from being merged.

@ordian ordian added A0-pleasereview 🤓 Pull request needs code review. A2-insubstantial 👶 Pull request requires no code review (e.g., a sub-repository hash update). M1-ci 🙉 Continuous integration. labels May 19, 2019
@ordian ordian added this to the 2.6 milestone May 19, 2019
@ordian ordian added B0-patch-stable 🕷 Pull request should also be back-ported to the stable branch. B1-patch-beta 🕷🕷 labels May 19, 2019
@dvdplm
Copy link
Collaborator

dvdplm commented May 20, 2019

Maybe it's an unpopular opinion but I think we should keep cargo audit as it is now (i.e. let it block merges). I agree that orthogonal concerns blocking our workflow is annoying and frustrating; the logic is impeccable.
However, I worry that without it we will become complacent with failures and leave it "for another day". This time we got a PR with a fix within hours, which is exactly the kind of turn-around time we want for anything security related. It is a relatively rare occurrence and I think the frustration is worth it.

@dvdplm dvdplm requested review from folsen and kirushik May 20, 2019 07:24
kirushik
kirushik reviewed May 20, 2019
View reviewed changes
Copy link
Collaborator

@kirushik kirushik left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

EDIT: There was an incorrect statement here about our CI not reporting individual steps to the Github UI; it clearly does.

@dvdplm
Copy link
Collaborator

dvdplm commented May 20, 2019 

ID:	 RUSTSEC-2019-0003
Crate:	 protobuf
Version: 1.7.5
Date:	 2019-06-08
URL:	 https://github.com/stepancheg/rust-protobuf/issues/411
Title:	 Out of Memory in stream::read_raw_bytes_into()
Solution: upgrade to: >= 1.7.5, >= 2.6.0

Sigh. So now we have a bug in cargo-audit too?

@dvdplm
Copy link
Collaborator

dvdplm commented May 20, 2019 •
edited
Loading

On my machine and cargo-audit 0.5.2 the check passes. Upgrading to latest (0.6.1) also passes. Can we upgrade to latest perhaps?
I'm wrong, fails on latest cargo-audit as well. Double sigh.

@ordian
Copy link
Collaborator Author

ordian commented May 20, 2019

Arguments for making cargo audit optional:

  • cargo audit failure is highly unlikely to be introduced by a PR, since when a new dependency is introduced, it usually has the latest version
  • cargo audit can fail at any point in time, e.g. after a PR is merged (when a vulnerability is discovered)
  • sometimes it can take some time make cargo audit happy (like e.g. upgrading tokio or ethereum-types)
  • cargo audit has bugs too (^^^)
  • we still can see if it fails
    Screenshot from 2019-05-20 10-37-53

@dvdplm
Copy link
Collaborator

dvdplm commented May 20, 2019

@ordian you are not wrong (at all) and I'm not going to make a fuss about this but I still suspect we'll let things slip through. I'd rather keep it.

@dvdplm dvdplm merged commit 67e75e1 into master May 20, 2019
@ordian ordian deleted the ao-make-cargo-audit-optional branch May 20, 2019 09:59
ordian added a commit that referenced this pull request May 20, 2019
@ordian
Merge branch 'master' into ao-upgrade-ethereum-types
40240cb 
* master:
  [CI] allow cargo audit to fail (#10676)
dvdplm added a commit that referenced this pull request May 21, 2019
@dvdplm
Merge branch 'master' into dp/chore/switch-to-new-parity-crypto
02eff51 
* master:
  Remove support for hardware wallets (#10678)
  [CI] allow cargo audit to fail (#10676)
This was referenced Jun 11, 2019
Merged
Merged
soc1c pushed a commit that referenced this pull request Jun 11, 2019
@ordian @soc1c
[CI] allow cargo audit to fail (#10676)
3718d14 
* [CI] allow cargo audit to fail

* [.gitlab-ci.yml] add a comment about cargo audit

* [Cargo.lock] cargo update -p protobuf
soc1c pushed a commit that referenced this pull request Jun 11, 2019
@ordian @soc1c
[CI] allow cargo audit to fail (#10676)
1ccee2d 
* [CI] allow cargo audit to fail

* [.gitlab-ci.yml] add a comment about cargo audit

* [Cargo.lock] cargo update -p protobuf
soc1c added a commit that referenced this pull request Jun 11, 2019
@soc1c
backports for stable 2.4.7 (#10736)
d6c5546 
* version: bump stable to 2.4.7

* [CI] allow cargo audit to fail (#10676)

* [CI] allow cargo audit to fail

* [.gitlab-ci.yml] add a comment about cargo audit

* [Cargo.lock] cargo update -p protobuf

* new image (#10673)

* Update publishing (#10644)

* docker images are now built on k8s: test run

* copy check_sync.sh in build-linux job

* copy scripts/docker/hub/* in build-linux job

* removed cache var

* cleanup, no more nightly dockers

* cleanup in dockerfile

* some new tags

* removed sccsche debug log, cleanup

* no_gits, new artifacts dir, changed scripts. Test run.

* define version once

* one source for TRACK

* stop kovan onchain updates

* moved changes for two images to a new branch

* rename Dockerfile

* no need in libudev-dev

* enable lto for release builds (#10717)

* Use RUSTFLAGS to set the optimization level (#10719)

* Use RUSTFLAGS to set the optimization level

Cargo has a [quirk]() in how configuration settings are propagated when `cargo test` runs: local code respect the settings in `[profile.test]` but all dependencies use the `[profile.dev]` settings. Here we force `opt-level=3` for all dependencies.

* Remove unused profile settings

* Maybe like this?

* Turn off incremental compilation

* Remove colors; try again with overflow-checks on

* Use quiet CI machine

* Turn overflow checking back on

* Be explicit about what options we use

* Remove "quiet machine" override

* ethcore: enable ECIP-1054 for classic (#10731)

* config: enable atlantis on ethereum classic

* config: enable atlantis on morden classic

* config: enable atlantis on morden classic

* config: enable atlantis on kotti classic

* ethcore: move kotti fork block to 0xAEF49

* ethcore: move morden fork block to 0x4829BA

* ethcore: move classic fork block to 0x81B320

* remove trailing comma

* remove trailing comma

* fix chainspec

* ethcore: move classic fork block to 0x7fffffffffffffff
soc1c added a commit that referenced this pull request Jun 11, 2019
@soc1c
backports for beta 2.5.2 (#10737)
ecbafb2 
* version: bump beta to 2.5.2

* [CI] allow cargo audit to fail (#10676)

* [CI] allow cargo audit to fail

* [.gitlab-ci.yml] add a comment about cargo audit

* [Cargo.lock] cargo update -p protobuf

* Reset blockchain properly (#10669)

* delete BlockDetails from COL_EXTRA

* better proofs

* added tests

* PR suggestions

* new image (#10673)

* Update publishing (#10644)

* docker images are now built on k8s: test run

* copy check_sync.sh in build-linux job

* copy scripts/docker/hub/* in build-linux job

* removed cache var

* cleanup, no more nightly dockers

* cleanup in dockerfile

* some new tags

* removed sccsche debug log, cleanup

* no_gits, new artifacts dir, changed scripts. Test run.

* define version once

* one source for TRACK

* stop kovan onchain updates

* moved changes for two images to a new branch

* rename Dockerfile

* no need in libudev-dev

* enable lto for release builds (#10717)

* Use RUSTFLAGS to set the optimization level (#10719)

* Use RUSTFLAGS to set the optimization level

Cargo has a [quirk]() in how configuration settings are propagated when `cargo test` runs: local code respect the settings in `[profile.test]` but all dependencies use the `[profile.dev]` settings. Here we force `opt-level=3` for all dependencies.

* Remove unused profile settings

* Maybe like this?

* Turn off incremental compilation

* Remove colors; try again with overflow-checks on

* Use quiet CI machine

* Turn overflow checking back on

* Be explicit about what options we use

* Remove "quiet machine" override

* ethcore: enable ECIP-1054 for classic (#10731)

* config: enable atlantis on ethereum classic

* config: enable atlantis on morden classic

* config: enable atlantis on morden classic

* config: enable atlantis on kotti classic

* ethcore: move kotti fork block to 0xAEF49

* ethcore: move morden fork block to 0x4829BA

* ethcore: move classic fork block to 0x81B320

* remove trailing comma

* remove trailing comma

* fix chainspec

* ethcore: move classic fork block to 0x7fffffffffffffff
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@tomaka tomaka tomaka approved these changes

@kirushik kirushik kirushik approved these changes

@dvdplm dvdplm dvdplm approved these changes

@folsen folsen Awaiting requested review from folsen

Assignees
No one assigned
Labels
A0-pleasereview 🤓 Pull request needs code review. A2-insubstantial 👶 Pull request requires no code review (e.g., a sub-repository hash update). B0-patch-stable 🕷 Pull request should also be back-ported to the stable branch. M1-ci 🙉 Continuous integration.
Projects
None yet
Milestone
2.6
Development

Successfully merging this pull request may close these issues.
4 participants
@ordian @dvdplm @kirushik @tomaka