search

shipwright-io / build

Shipwright - a framework for building container images on Kubernetes
https://shipwright.io
Apache License 2.0
672 stars 113 forks source link

[FEATURE] spec.buildSteps[4].securityContext.runAsUser: Invalid value: "string" #1354

Open cmoulliard opened 1 year ago

cmoulliard commented 1 year ago

Is there an existing issue for this?

Kubernetes Version

1.27

Shipwright Version

Shipwright: v0.11.0 Tekton: v0.48.0

Current Behavior

When we would like to use $(params.USER_ID) part of the following YAML syntax of the BuildStrategy

buildSteps:
    - name: build-and-push
      image: $(params.CNB_BUILDER_IMAGE)
      imagePullPolicy: Always
      securityContext:
        runAsUser: $(params.USER_ID)
        runAsGroup: $(params.GROUP_ID)
      command: ["/cnb/lifecycle/builder"]
      args:
        - "-app=$(workspaces.source.path)/$(params.SOURCE_SUBPATH)"
        - "-layers=/layers"
        - "-group=/layers/group.toml"
        - "-plan=/layers/plan.toml"

we got this error: * spec.buildSteps[4].securityContext.runAsUser: Invalid value: "string": spec.buildSteps[4].securityContext.runAsUser in body must be of type integer: "string"

Ideally we should be able to pass a parameter which is nextconverted to an int

        runAsUser: 1001
        runAsGroup: 1000

Expected Behavior

No response

Steps To Reproduce

No response

Anything else?

No response

SaschaSchwarze0 commented 1 year ago

Related discussion: https://kubernetes.slack.com/archives/C019ZRGUEJC/p1691586801042639

qu1queee commented 1 year ago

From refinement, adding a priority to this issue and help wanted label. We also discussed on the current K8S version and the need for an update (not related to this issue)

qu1queee commented 1 year ago

@SaschaSchwarze0 does this issue qualifies for a bug (without reading the slack conversation) ?

SaschaSchwarze0 commented 1 year ago

@SaschaSchwarze0 does this issue qualifies for a bug (without reading the slack conversation) ?

Imo not, I consider this a feature request. We probably have not documented today in all possible details where a param can be used and where not, which is also something we can do. So far, we've only documented the limitation where config and secret references can be used (in https://github.com/shipwright-io/build/blob/main/docs/build.md#defining-paramvalues).

cmoulliard commented 7 months ago

Imo not, I consider this a feature request.

Ideally such a parameter should be set from a step executed previously aka what we can using Tekton task results

SaschaSchwarze0 commented 7 months ago

Imo not, I consider this a feature request.

Ideally such a parameter should be set from a step executed previously aka what we can using Tekton task results

This would not be possible with Shipwright's current design to use a TaskRun to run a BuildRun. We basically have only one Pod and container run-as values are immutable. Or basically, at the time a step would write that result, the containers of all other steps are already started (and are handing in Tekton's entrypoint).

cmoulliard commented 7 months ago

This would not be possible with Shipwright's current design to use a TaskRun to run a BuildRun. We basically have only one Pod and container run-as values are immutable. Or basically, at the time a step would write that result, the containers of all other steps are already started (and are handing in Tekton's entrypoint).

Can a temporary solution be that a build A store the result using a volume or configMap and that an ENV var or param is set from the volume or ConfigMap within the build B ?

SaschaSchwarze0 commented 7 months ago

This would not be possible with Shipwright's current design to use a TaskRun to run a BuildRun. We basically have only one Pod and container run-as values are immutable. Or basically, at the time a step would write that result, the containers of all other steps are already started (and are handing in Tekton's entrypoint).

Can a temporary solution be that a build A store the result using a volume or configMap and that an ENV var or param is set from the volume or ConfigMap within the build B ?

I don't think so. The purpose of a Build is to produce a container image, and not results for another build. What is the use case behind a dynamic runAs configuration of a step that is determined by a previous step?

What would actually work is the emptyDir combined with su: Step 1 can write a file to that emptyDir with the user and group. Step 2 would run as root and would run the desired command through su, would certainly need more privileges then.

cmoulliard commented 7 months ago

I don't think so. The purpose of a Build is to produce a container image, and not results for another build. What is the use case behind a dynamic runAs configuration of a step that is determined by a previous step?

To build a container image using buildpack and extensions, it is needed to know in advance what the UID/GUID are - see: https://github.com/redhat-buildpacks/catalog/blob/main/tekton/task/buildpacks-extension-check/01/buildpacks-extension-check.yaml as such UID/GUID could be different between the UBI builder images (red hat vs paketo vs tanzu etc) and will allow kanilo to write layers created within the mounted volume

SaschaSchwarze0 commented 7 months ago

I don't think so. The purpose of a Build is to produce a container image, and not results for another build. What is the use case behind a dynamic runAs configuration of a step that is determined by a previous step?

To build a container image using buildpack and extensions, it is needed to know in advance what the UID/GUID are - see: https://github.com/redhat-buildpacks/catalog/blob/main/tekton/task/buildpacks-extension-check/01/buildpacks-extension-check.yaml as such UID/GUID could be different between the UBI builder images (red hat vs paketo vs tanzu etc) and will allow kanilo to write layers created within the mounted volume

So, you create a build strategy where the image itself is dynamic coming from a parameter. And depending on which image is used, the runAsUser/runAsGroup would be different. But, you do not need to define that on the steps that use the image itself (because they would simply use the correct user and group as defined in the image itself), but you need to define those on a step that runs a different image (kaniko). All correct ?

Beside the su option, another option would be to use different build strategies.

adambkaplan commented 4 months ago

I think we need to revisit the requirement. We're a bit hesitant to support dynamic parameters for the securityContext field, given that can have impacts on other parts of the pod spec (ex: added capabilities) and required pod security standard.

That said, in theory we can implement a version of the BuildStep API that lets these be set as an IntOrString type, which won't break our YAML/OpenAPI schema. Dependent golang code would need to be updated.