Bug 1748061 - OpenShift 3.11 Issue with large Input Parameters when processing a Template
Summary: OpenShift 3.11 Issue with large Input Parameters when processing a Template
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: OpenShift Container Platform
Classification: Red Hat
Component: Templates
Version: 3.11.0
Hardware: Unspecified
OS: Unspecified
unspecified
medium
Target Milestone: ---
: 4.4.0
Assignee: Gabe Montero
QA Contact: XiuJuan Wang
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2019-09-02 15:28 UTC by Christian Stark
Modified: 2020-05-04 11:14 UTC (History)
6 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Cause: When using the '--param-file' option with template processing commands like 'oc new-app' or 'oc process', if the file was greater than 64K in size, it would not be fully read in. Consequence: The oc based template processing with '--param-file' would fail. Fix: Updated the code to check the size of the file specified by '--process-file' and augment the parameters used to read the file in the read the entire file. Result: The oc based template processing with '--param-file' pointing to files greater than 64K now works.
Clone Of:
Environment:
Last Closed: 2020-05-04 11:13:32 UTC
Target Upstream Version:
gmontero: needinfo-
gmontero: needinfo-


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
Github openshift oc pull 177 0 'None' closed Bug 1748061: copy gotdotenv and add ability to override underlying scanner buffer … 2020-04-21 06:40:55 UTC
Red Hat Product Errata RHBA-2020:0581 0 None None None 2020-05-04 11:14:08 UTC

Description Christian Stark 2019-09-02 15:28:53 UTC
Description of problem:

Customer needs to pass a large value into a Template

However when executing the template we receive this error:

Cannot read variables from file "PARAMS": bufio.Scanner: token too long


This is how to reproduce it

for l in $(seq 20000 10000 150000); do
    echo -n "BLA=" > PARAMS
    tr -cd '[:alnum:]' < /dev/urandom | head -c $l | base64 -w 0 >> PARAMS

    cat << EOF | oc process -f - --param-file=PARAMS --local >/dev/null && echo "OK $l" || echo "Fail: $l" 
apiVersion: v1
kind: Template
parameters:
  - name: BLA
objects:
- apiVersion: v1
  kind: Secret
  metadata:
    name: BLAtest
  data:
    password: \${BLA}
  type: Opaque
EOF
done

RESULT:
$ ./test.sh 
OK 20000
OK 30000
OK 40000
error: Cannot read variables from file "PARAMS": bufio.Scanner: token too long
Fail: 50000



Version-Release number of selected component (if applicable):

3.11

How reproducible:

see description,


Additional info:

it seems the limit comes from golang


customer is unhappy with the workaround


Have the user supply the name of a secret/configmap that will contain the cert and make them create the secret/configmap separately.  (and have the template reference the configmap/secret name they provided as needed for mounting/etc).

Comment 1 Adam Kaplan 2019-09-04 13:41:06 UTC
Parameters are meant to process reasonably sized items in memory. In your test the script is generating 50000 random characters that are base64 encoded, more than likely hitting the default limit of 64kB of data to read in.

Please note that using a ConfigMap or Secret has its limits - etcd only supports objects less than 1MB in size. Any Secret or ConfigMap therefore must have less than 1MB of data - anything larger and the user needs to create a Persistent Volume.

Comment 2 Christian Koep 2019-11-08 12:06:11 UTC
Hi Adam,

thank you for your feedback. I'd like to re-open the discussion on the behaviour with a bit more context.

The real topic of this Bugzilla is to investigate a mismatch between the `--param-file` and `--param` options for the `oc process` command.

Let's assume that [1] is a long TLS certificate chain (base64 encoded) saved in a filed called "PARAMS_b64" which shall be included into the following `ConfigMap`:

~~~ 
$ cat configmap-template.yaml
apiVersion: v1
kind: Template
metadata:
  name: security-template
  annotations:
    description: "Description"
objects:
  - metadata:
      name: configmap-test
    apiVersion: v1
    data:
    caTLSRootCert: ${TestCert} 
    kind: ConfigMap
parameters:
  - name: TestCert
    require: true
~~~

Now, the following command does not work:

~~~
$ oc process -f configmap-template.yaml --param-file=PARAMS_b64
error: Cannot read variables from file "PARAMS_b64": bufio.Scanner: token too long
~~~

However, this does work:

~~~
$ oc process -f configmap-template.yaml --param=$(cat PARAMS_b64) | oc apply -f -
configmap/configmap-test configured
~~~

This behaviour is true for OpenShift Container Platform version 3.11 and 4.2 (potentially all others as well).

Please can you advise on whether or not this is working as intended in your opinion? We would like to explore if this behaviour should be adjusted (at the very least in OpenShift 4.x).

Thank you very much and kind regards,
Christian

---

[1] - https://pastebin.com/raw/pz08tSMJ

Comment 3 Adam Kaplan 2019-11-08 13:53:43 UTC
Thank you, Christian, for your feedback and reproducer. I think this does warrant further investigation since `--param` and `--param-file` inputs should be able to be processed equally.

Setting target to 4.4.0 given the priority and severity.

Comment 5 Gabe Montero 2019-11-20 17:05:41 UTC
Did some quick research in the code.  Ultimately, this is currently a function of the fact that the newapp helpers in `oc` are using "github.com/joho/godotenv"

First, see https://github.com/openshift/oc/blob/master/pkg/helpers/newapp/app/env.go#L164 and the `gotdotenv.Read(filename)` call 

gotdotenv is using the default bufio.NewScanner which means a max size of 64K ... it does not provide a means to override that.

To fix this we would need to move off of gotdotenv in newapp.  Not overly onerous, but not a couple of lines of code neither (the current gotdotenv path is 50 to 100 lines).  
Plus regression testing for all the paths using these newapp helpers.

Given Christians pretty easy workaround wrt --param=$(cat PARAMS_b64), and the subsequent low severity of this, and where we are with templates and oc new-app more and more being in maintenance mode,
I would content we close this as WONTFIX.

Thoughts gentlemen?

Comment 6 Christian Koep 2019-11-20 18:41:32 UTC
Thanks for your work on this Gabe. 

The presented "work around" to demonstrate this bug / inconsistency might not be applicable in any case.

If possible, could you please allow me some time to discuss this internally before closing this again as WONTFIX?

Thanks and kind regards,
Christian

Comment 7 Gabe Montero 2019-11-20 18:46:45 UTC
Sure we can sit tight for a bit Christian ... and oops on the misdirected needsinfo :-)

Comment 8 Gabe Montero 2019-11-20 20:54:51 UTC
Conversely waiting on takes from Adam, et. al. on our end wrt should we spend time on this given there is an open customer case with this, regardless of priority

Comment 9 Gabe Montero 2019-11-21 17:07:07 UTC
wrt the cost of fixing this ... I've got a prototype PR up of what that might look like:  https://github.com/openshift/oc/pull/177

Comment 10 Christian Koep 2019-11-22 10:49:39 UTC
Hi Gabe,

thanks for your work on the PR!

As mentioned before, I'm working to give you a bit more information on why a fix makes sense from the customer perspective. Please stay tuned for more to come from that front.

From my point of view, making the behaviour of `--param-file` and `param` consistent would improve our product regardless of any specific use case (simply to improve the developer experience, which is crucial).

Kind regards,
Christian

Comment 17 XiuJuan Wang 2019-12-16 06:50:41 UTC
#! /bin/bash

for l in $(seq 20000 10000 300000); do
    echo -n "BLA=" > PARAMS
    tr -cd '[:alnum:]' < /dev/urandom | head -c $l | base64 -w 0 >> PARAMS

    cat << EOF | 4.4/oc process -f - --param-file=PARAMS --local >/dev/null && echo "OK $l" || echo "Fail: $l" 
apiVersion: v1
kind: Template
parameters:
  - name: BLA
objects:
- apiVersion: v1
  kind: Secret
  metadata:
    name: BLAtest
  data:
    password: \${BLA}
  type: Opaque
EOF
done

The result:
OK 20000
OK 30000
OK 40000
OK 50000
OK 60000
OK 70000
OK 80000
OK 90000
OK 100000
OK 110000
OK 120000
OK 130000
OK 140000
OK 150000

Could create resource with large Input Parameters without error.
$ oc version 
Client Version: 4.4.0-0.nightly-2019-12-14-103510
Server Version: 4.4.0-0.nightly-2019-12-14-103510
Kubernetes Version: v1.16.2

Also tried more biger Input Parameters 3000000, create succeed.

Comment 20 errata-xmlrpc 2020-05-04 11:13:32 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHBA-2020:0581


Note You need to log in before you can comment on or make changes to this bug.