Skip to content

Forbid unsafe code in most module#1018

Merged
olix0r merged 6 commits intomainfrom
ver/forbid-unsafe
May 25, 2021
Merged

Forbid unsafe code in most module#1018
olix0r merged 6 commits intomainfrom
ver/forbid-unsafe

Conversation

@olix0r
Copy link
Member

@olix0r olix0r commented May 24, 2021

This change adds the forbid(unsafe_code) directive to most modules,
ensuring that these modules are "safe". There are three modules this
cannot apply to:

  • duplex -- due to the implementation of BufMut for CopyBuf; and
  • proxy::transport -- due to socket option interactions.
  • system -- for system-level counter access

Furthermore, this change adds deny(warnings) directives to a few
modules that were missing it.

This change adds the `forbid(unsafe_code)` directive to most modules,
ensuring that these modules are "safe". There are three modules this
cannot apply to:

- `app::core` -- due to the `process_start_time` metric implementation;
- `duplex` -- due to the implementation of `BufMut` for `CopyBuf`; and
- `proxy::transport` -- due to socket option interactions.

Furthermore, this change adds `deny(warnings)` directives to a few
modules that were missing it.
@olix0r olix0r requested a review from a team May 24, 2021 22:01
Copy link
Contributor

@hawkw hawkw left a comment

Choose a reason for hiding this comment

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

lgtm!

it may be worth a top-level comment in the crates that require unsafe code explaining why they can't forbid unsafe? but, not a hard blocker.

@olix0r olix0r merged commit c188444 into main May 25, 2021
@olix0r olix0r deleted the ver/forbid-unsafe branch May 25, 2021 01:20
olix0r added a commit to linkerd/linkerd2 that referenced this pull request May 27, 2021
* Controller clients of components with more than one replica could fail
  to drive all connections to completion. This could result in timeouts
  showing up in logs, but would not have prevented proxies from
  communicating with controllers. #6146
* linkerd/linkerd2-proxy#992 made the `l5d-dst-override` header required
  for ingress-mode proxies. This behavior has been reverted so that
  requests without this header are forwarded to their original
  destination.
* OpenCensus trace spans for HTTP requests no longer include query
  parameters.

---

* ci: Update/pin action dependencies (linkerd/linkerd2-proxy#1012)
* control: Ensure endpoints are driven to readiness (linkerd/linkerd2-proxy#1014)
* Make span name without query string (linkerd/linkerd2-proxy#1013)
* ingress: Restore original dst address routing (linkerd/linkerd2-proxy#1016)
* ci: Restict permissions in Actions (linkerd/linkerd2-proxy#1019)
* Forbid unsafe code in most module (linkerd/linkerd2-proxy#1018)
olix0r added a commit to linkerd/linkerd2 that referenced this pull request May 27, 2021
* Controller clients of components with more than one replica could fail
  to drive all connections to completion. This could result in timeouts
  showing up in logs, but would not have prevented proxies from
  communicating with controllers. #6146
* linkerd/linkerd2-proxy#992 made the `l5d-dst-override` header required
  for ingress-mode proxies. This behavior has been reverted so that
  requests without this header are forwarded to their original
  destination.
* OpenCensus trace spans for HTTP requests no longer include query
  parameters.

---

* ci: Update/pin action dependencies (linkerd/linkerd2-proxy#1012)
* control: Ensure endpoints are driven to readiness (linkerd/linkerd2-proxy#1014)
* Make span name without query string (linkerd/linkerd2-proxy#1013)
* ingress: Restore original dst address routing (linkerd/linkerd2-proxy#1016)
* ci: Restict permissions in Actions (linkerd/linkerd2-proxy#1019)
* Forbid unsafe code in most module (linkerd/linkerd2-proxy#1018)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants