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

Fix policy-server image building failures #346

Closed
viccuad opened this issue Oct 19, 2022 · 5 comments
Closed

Fix policy-server image building failures #346

viccuad opened this issue Oct 19, 2022 · 5 comments
Assignees

Comments

@viccuad
Copy link
Member

viccuad commented Oct 19, 2022

See https://github.com/kubewarden/policy-server/actions/runs/3273562423/jobs/5386064156#step:10:777

@viccuad viccuad changed the title Fix image building failures Fix policy-server image building failures Oct 19, 2022
@viccuad viccuad self-assigned this Oct 19, 2022
@flavio
Copy link
Member

flavio commented Oct 20, 2022

I noticed the build fails with:

error: failed to run custom build command for `openssl-sys v0.9.76`

We should not require the openssl system library, we're building using rustls. If I were you, I would check via cargo tree who is pulling in this dependency. Maybe there's some feature flag we have to enable

@viccuad
Copy link
Member Author

viccuad commented Oct 20, 2022

native-tls is pulled by reqwest. This happens because the default reqwest features include native-tls.

reqwest is needed by policy-fetcher, oci-distribution, sigstore, tough, oauth2. These libraries pull reqwest with the default features, and don't make use of default-features = false.

Features are additive, there's no way to drop the native-tls feature and enable only rustls-tls in all the times that reqwest is imported (see here). We would need to amend or fork the libs that depend on reqwest. Tried also [patches.cargo-io] et al, but don't support changing feature sets.

@flavio
Copy link
Member

flavio commented Oct 20, 2022

We used to have the rusttls feature handled by all these libraries, maybe one of them dropped/broke that by mistake. Let me take a closer look

@viccuad
Copy link
Member Author

viccuad commented Oct 20, 2022

openssl-sys and native-tls was introduced in the last PR to policy-server, the one that bumps policy-evaluator -> ..-> sigstore-rs.

Tried patching sigstore-rs so it enables reqwest with rustls-tls feature (and consuming it):
viccuad/sigstore-rs@5cd421b

[features]
default = ["native-tls"]
native-tls = ["oci-distribution/native-tls", "openidconnect/native-tls", "reqwest/native-tls"]
rustls-tls = ["oci-distribution/rustls-tls", "openidconnect/rustls-tls", "reqwest/rustls-tls"]
vic@viccuad2 ~/suse/kw/policy-server (main *$%>)$ cargo tree -i openssl-sys
    Updating git repository `https://github.com/viccuad/policy-evaluator`
    Updating git repository `https://github.com/viccuad/policy-fetcher`
    Updating git repository `https://github.com/viccuad/sigstore-rs`
openssl-sys v0.9.76
├── native-tls v0.2.10
│   ├── hyper-tls v0.5.0
│   │   └── reqwest v0.11.12
│   │       ├── oauth2 v4.2.3
│   │       │   └── openidconnect v2.3.2
│   │       │       └── sigstore v0.5.2 (https://github.com/viccuad/sigstore-rs?branch=reqwest-rustls-tls#5cd421b5)
│   │       │           └── policy-fetcher v0.7.12 (https://github.com/viccuad/policy-fetcher?branch=rustls#b930e554)
│   │       │               └── policy-evaluator v0.4.10 (https://github.com/viccuad/policy-evaluator?branch=rustls#620216b0)
│   │       │                   └── policy-server v1.1.2 (/home/vic/suse/kw/policy-server)
│   │       ├── oci-distribution v0.9.3
│   │       │   ├── policy-fetcher v0.7.12 (https://github.com/viccuad/policy-fetcher?branch=rustls#b930e554) (*)
│   │       │   └── sigstore v0.5.2 (https://github.com/viccuad/sigstore-rs?branch=reqwest-rustls-tls#5cd421b5) (*)
│   │       ├── policy-fetcher v0.7.12 (https://github.com/viccuad/policy-fetcher?branch=rustls#b930e554) (*)
│   │       ├── sigstore v0.5.2 (https://github.com/viccuad/sigstore-rs?branch=reqwest-rustls-tls#5cd421b5) (*)
│   │       └── tough v0.12.5
│   │           └── sigstore v0.5.2 (https://github.com/viccuad/sigstore-rs?branch=reqwest-rustls-tls#5cd421b5) (*)
│   ├── reqwest v0.11.12 (*)
│   └── tokio-native-tls v0.3.0
│       ├── hyper-tls v0.5.0 (*)
│       └── reqwest v0.11.12 (*)
└── openssl v0.10.42
    └── native-tls v0.2.10 (*)

Sadly, native-tls still gets pulled in. I'm not sure if patching sigstore-rs helped, and if other crate is still pulling it. But they all seem to use default-features = false

@flavio
Copy link
Member

flavio commented Oct 20, 2022

Moved to blocked, this is going to be fixed once sigstore/sigstore-rs#146 is merged and a new release is tagged.

@viccuad viccuad removed their assignment Oct 21, 2022
flavio added a commit to flavio/policy-server that referenced this issue Oct 21, 2022
Ensure the latest version of sigstore-rs is used, this prevents openssl
from becoming a dependency.

This fixes kubewarden#346

Signed-off-by: Flavio Castelli <fcastelli@suse.com>
@flavio flavio closed this as completed in bebbfcd Oct 21, 2022
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

No branches or pull requests

2 participants