GoogleContainerTools / GoogleContainerTools/skaffold

AutoPull Images in cacheFrom Should not be default behavior

Open
#6,925 4 comments 0 reactions 0 assignees View on GitHub
area/build area/cache area/s3c build/docker kind/design discussion planning/H1-2022 priority/p1
Dominant language
Go
Stars
15.9k
Forks
1.7k
Avg merge
3d 9h
Merged PRs (30d)
10

Description

### Expected behavior
PR https://github.com/GoogleContainerTools/skaffold/pull/1495 introduced pre-pulling behavior as the default when the cacheFrom keyword is used (with no option to disable this, I believe).

In my humble opinion, this is a mistake and honestly can introduce security risks.

Let me explain. If the first or second command of a Dockerfile is something like `apt-get update && apt-get dist-upgrade -y`, the user might think that they are actually getting the latest security updates periodically. Now, let's say the dockerfile only changes towards the end (changes default shell for example). In skaffold's current behavior, if I want to use cacheFrom to speed up builds, it downloads the `latest` image, which from a Dockerfile perspective only shows a change on the last line. **it's possible that another security update will never be run again until the cache images are invalidated!**

The way we previously handled this with our docker builds in gitlab runners was something like this

```bash
docker pull :latest
docker image prune -a --force --filter "until=12h"
docker build --cache-from :latest .....
```

In this way, we would invalidate the cache anytime it had cooled off for more than 12 hours. With the current implementation of skaffold, this isn't really possible.

The following file will always pull the cache even with **tryImportMissing: false** (which is likely an unrelated command, but confusing all the same)

```yaml
apiVersion: skaffold/v2beta24
kind: Config
build:
artifacts:
- image: myimage
context: .
docker:
cacheFrom:
- "myimage:latest"
local:
useBuildkit: true
push: false
concurrency: 0
tryImportMissing: false
```

A couple of possible solutions from (my) preferred to least preferred

- Skaffold remove this "always pull" cacheFrom behavior as the default policy with an **option** to enable it
- Skaffold leave the default behavior as is, but allow an **option** to disable it on a per image basis
- evaluate tryImportMissing ...and don't import cache files if they are...missing
- Use Patching to remove cacheFrom stanzas conditionally based on some variable - this is painful and devs are likely going to forget to set variables for this
- Add an environment variable near the top of the Dockerfiles with the date, that changes every 24 hours, and thus invalidates the cache - this is error prone since devs will forget to put this in their files. Also, we will now waste time pulling a cache image that we might know is invalid.
- Create a secondary service that intentionally invalidates my cache every night (e.g., `docker pull scratch && docker tag scratch :latest && docker push :latest`) - This is getting pretty gross in my opinion
- Stop using caching completely - Unfortunately, we can't afford to do this...our images are time-consuming to build

### Information

- Skaffold version: v1.35.0
- Operating system: Ubuntu, 18.04.6 LTS
- Installed via: skaffold.dev

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.