[BUG] flytectl fails to unmarshal execution spec with certain input key

[honnix]

Describe the bug

If execution spec has input parameter named as n, y, no, yes, etc. flytectl fails to unmarshal the spec correctly. As an example:

inputs:
  n: 42

flytectl wil fail with

Error: no matching type for [false]
{"json":{},"level":"error","msg":"no matching type for [false]","ts":"2023-03-14T10:48:56-04:00"}

After a quick investigation, this is because of using sigs.k8s.io/yaml in flytectl: https://github.com/flyteorg/flytectl/blob/cea39d9bdf2476f9a5313d1bf19bf08b3923237a/cmd/create/execution_util.go#L13. This yaml library depends on go-yaml v2, which does not support YAML 1.2 specification, see go-yaml/yaml#214. If an input argument is named as n, when creating an execution via a execution spec file, that argument must be quoted as "n", otherwise flytectl would fail because when unmarshalling n is parsed as false as the input parameter name.

Expected behavior

flytectl should correctly unmarshal the execution spec that is compliant with YAML 1.2 specification.

Additional context to reproduce

No response

Screenshots

No response

Are you sure this issue hasn't been raised already?

• Yes

Have you read the Code of Conduct?

• Yes

[honnix]

I tried replacing sigs.k8s.io/yaml with gopkg.in/yaml.v3 but that results in some test case failures, so they do behave in a different way when coming to unmarshaling. Seems to be related with default value in struct.

--- FAIL: TestCreateSuite (0.02s)
    --- FAIL: TestCreateSuite/Test_CreateTaskExecution (0.00s)
        execution_test.go:195:
            	Error Trace:	execution_test.go:195
            	            				AdminServiceClient.go:47
            	            				execution.go:219
            	            				execution_test.go:201
            	Error:      	Should be true
            	Test:       	TestCreateSuite/Test_CreateTaskExecution
            	Messages:   	project:"dummyProject" domain:"dummyDomain" spec:<launch_plan:<resource_type:TASK project:"dummyProject" domain:"dummyDomain" name:"task1" version:"v2" > metadata:<principal:"sdk" > >
        execution_test.go:36: PASS:	GetTask(*context.emptyCtx,string)
        execution_test.go:36: PASS:	CreateExecution(*context.emptyCtx,string)

Now I'm not sure how we could fix this in flytectl. Maybe we will need to wait for kubernetes-sigs/yaml#76

[honnix]

Note that flytectl also uses unique functions from sigs.k8s.io/yaml, so we will not be able to fully replace that with gopkg.in/yaml.v3.

[pmahindrakar-oss]

We have dependency in the printer functionality which uses specific function JSONToYaml which is not there in other library . We could just do yaml.Marshal(v) in the printer but dont know more implications of it .

I saw just a project get was dumping unnecessary hidden fields from the proto with gopkg library.

        xxx_nounkeyedliteral: {}
        xxx_unrecognized: []
        xxx_sizecache: 0

eg:

flytectl get projects -o yaml
- message:
    id: wpcustomnode
    name: wpcustomnode
    domains:
        - id: development
          name: development
          xxx_nounkeyedliteral: {}
          xxx_unrecognized: []
          xxx_sizecache: 0
        - id: staging
          name: staging
          xxx_nounkeyedliteral: {}
          xxx_unrecognized: []
          xxx_sizecache: 0
        - id: production
          name: production
          xxx_nounkeyedliteral: {}
          xxx_unrecognized: []
          xxx_sizecache: 0
    description: Project to test running workflows on customnodes
    labels:
        values:
            app: flyte
        xxx_nounkeyedliteral: {}
        xxx_unrecognized: []
        xxx_sizecache: 0
    state: 0
    xxx_nounkeyedliteral: {}
    xxx_unrecognized: []
    xxx_sizecache: 0

Which might be the reason the unit tests fail too.

We could probably wait until the fix goes in sigs unless we can resolve this issue with gopkg library

[github-actions]

Hello 👋, this issue has been inactive for over 9 months. To help maintain a clean and focused backlog, we'll be marking this issue as stale and will engage on it to decide if it is still applicable.
Thank you for your contribution and understanding! 🙏