build icon indicating copy to clipboard operation
build copied to clipboard

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

Open cmoulliard opened this issue 2 years ago • 12 comments

Is there an existing issue for this?

  • [X] I have searched the existing issues

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

cmoulliard avatar Aug 09 '23 13:08 cmoulliard

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

SaschaSchwarze0 avatar Aug 09 '23 13:08 SaschaSchwarze0

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 avatar Aug 23 '23 13:08 qu1queee

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

qu1queee avatar Aug 23 '23 13:08 qu1queee

@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).

SaschaSchwarze0 avatar Aug 23 '23 15:08 SaschaSchwarze0

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

cmoulliard avatar Apr 15 '24 08:04 cmoulliard

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).

SaschaSchwarze0 avatar Apr 15 '24 08:04 SaschaSchwarze0

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 ?

cmoulliard avatar Apr 15 '24 08:04 cmoulliard

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.

SaschaSchwarze0 avatar Apr 15 '24 09:04 SaschaSchwarze0

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

cmoulliard avatar Apr 15 '24 10:04 cmoulliard

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.

SaschaSchwarze0 avatar Apr 15 '24 12:04 SaschaSchwarze0

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.

adambkaplan avatar Jul 11 '24 14:07 adambkaplan

After reviewing the issue as well as official k8s reference of SecurityContext, runAsGroup and runAsUser are integer type. With that in place, wouldn't it better to keep integer as the expected type for SecurityContext to remain comform with k8s. Instead, why wouldn't we expand the Strategy Parameters to string | array | int ?

Just an idea

rxinui avatar Apr 25 '25 08:04 rxinui