Skip to content

CF-3931 : Reject mis-shaped --statement-defaults instead of silently ignoring it #3397

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Paras Negi (paras-negi-flink) wants to merge 3 commits into
base: main
Choose a base branch
from
Open

CF-3931 : Reject mis-shaped --statement-defaults instead of silently ignoring it #3397

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2391d66
CF-3931 : Reject mis-shaped --statement-defaults instead of silently …
paras-negi-flink Jul 6, 2026
599851a
CF-3931 : Reject trailing data and extra documents in strict defaults…
paras-negi-flink Jul 7, 2026
165a1e9
[CF-3931] Move --statement-defaults shape into a validation helper; a…
Aug 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 22 additions & 4 deletions internal/flink/command_environment_create.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package flink

import (
"bytes"
"encoding/json"
"fmt"
"os"
Expand All @@ -26,7 +27,7 @@ func (c *command) newEnvironmentCreateCommand() *cobra.Command {

cmd.Flags().String("kubernetes-namespace", "", "Kubernetes namespace to deploy Flink applications to.")
cmd.Flags().String("defaults", "", "JSON string defining the environment's Flink application defaults, or path to a file to read defaults from (with .yml, .yaml or .json extension).")
cmd.Flags().String("statement-defaults", "", "JSON string defining the environment's Flink statement defaults, or path to a file to read defaults from (with .yml, .yaml or .json extension).")
cmd.Flags().String("statement-defaults", "", `JSON string defining the environment's Flink statement defaults, or path to a file to read defaults from (with .yml, .yaml or .json extension). Expected shape: {"detached":{"flinkConfiguration":{...}},"interactive":{"flinkConfiguration":{...}}}.`)

@channingdong Channing Dong (channingdong) Jul 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest to:

  • Avoid specifying the expected shape here.
  • Have a dedicated function to perform validation + disallow unknown fields, and provide prompts/error out to users if the shape is invalid.

@channingdong Channing Dong (channingdong) Jul 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise the same decodeStrictJson() and decodeStrictYaml() will have to be applied everywhere for on-prem.

@paras-negi-flink Paras Negi (paras-negi-flink) Aug 2, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done — extracted parseStatementDefaults .
Also dropped the Expected shape string from both flag-help texts.

cmd.Flags().String("compute-pool-defaults", "", "JSON string defining the environment's Flink compute pool defaults, or path to a file to read defaults from (with .yml, .yaml or .json extension).")

addCmfFlagSet(cmd)
Expand Down Expand Up @@ -124,18 +125,18 @@ func parseDefaultsAsGenericType[T any](input, label string) (T, error) {
if err != nil {
return out, fmt.Errorf("failed to read %s defaults JSON file: %w", label, err)
}
err = json.Unmarshal(data, &out)
err = decodeStrictJson(data, &out)

case ".yaml", ".yml":
data, err = os.ReadFile(input)
if err != nil {
return out, fmt.Errorf("failed to read %s defaults YAML file: %w", label, err)
}
err = yaml.Unmarshal(data, &out)
err = decodeStrictYaml(data, &out)

default:
// inline JSON string
err = json.Unmarshal([]byte(input), &out)
err = decodeStrictJson([]byte(input), &out)
}

if err != nil {
Expand All @@ -144,6 +145,23 @@ func parseDefaultsAsGenericType[T any](input, label string) (T, error) {
return out, nil
}

// decodeStrictJson decodes JSON into out, rejecting keys that do not map to a
// known field. This surfaces mis-shaped input (for example the wrong nesting for
// --statement-defaults) instead of silently dropping it. Decoding into a map is
// unaffected, since a map has no unknown fields.
func decodeStrictJson(data []byte, out any) error {
decoder := json.NewDecoder(bytes.NewReader(data))
decoder.DisallowUnknownFields()
return decoder.Decode(out)
}

// decodeStrictYaml is the YAML counterpart of decodeStrictJson.
func decodeStrictYaml(data []byte, out any) error {
decoder := yaml.NewDecoder(bytes.NewReader(data))
decoder.KnownFields(true)
return decoder.Decode(out)
}
Comment on lines +172 to +199

@channingdong Channing Dong (channingdong) Jul 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment makes sense, we can absorb it and make it part of the validation function as I suggest above.

@paras-negi-flink Paras Negi (paras-negi-flink) Aug 2, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Handled — the strict decoders now reject trailing content.


func jsonMarshalHelper(v interface{}, label string) (string, error) {
data, err := json.Marshal(v)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion internal/flink/command_environment_update.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@

addCmfFlagSet(cmd)
cmd.Flags().String("defaults", "", "JSON string defining the environment's Flink application defaults, or path to a file to read defaults from (with .yml, .yaml or .json extension).")
cmd.Flags().String("statement-defaults", "", "JSON string defining the environment's Flink statement defaults, or path to a file to read defaults from (with .yml, .yaml or .json extension).")
cmd.Flags().String("statement-defaults", "", `JSON string defining the environment's Flink statement defaults, or path to a file to read defaults from (with .yml, .yaml or .json extension). Expected shape: {"detached":{"flinkConfiguration":{...}},"interactive":{"flinkConfiguration":{...}}}.`)

Check failure on line 24 in internal/flink/command_environment_update.go

View check run for this annotation

SonarQube-Confluent / SonarQube Code Analysis

Define a constant instead of duplicating this literal "statement-defaults" 3 times. 

[S1192] String literals should not be duplicated
 See more on https://sonarqube.confluent.io/project/issues?id=cli&pullRequest=3397&issues=421e0c7a-78de-4d19-b824-966a98720a70&open=421e0c7a-78de-4d19-b824-966a98720a70
cmd.Flags().String("compute-pool-defaults", "", "JSON string defining the environment's Flink compute pool defaults, or path to a file to read defaults from (with .yml, .yaml or .json extension).")
pcmd.AddOutputFlag(cmd)

Expand Down
2 changes: 1 addition & 1 deletion test/fixtures/output/flink/environment/create-help-onprem.golden
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ Usage:
Flags:
--kubernetes-namespace string REQUIRED: Kubernetes namespace to deploy Flink applications to.
--defaults string JSON string defining the environment's Flink application defaults, or path to a file to read defaults from (with .yml, .yaml or .json extension).
--statement-defaults string JSON string defining the environment's Flink statement defaults, or path to a file to read defaults from (with .yml, .yaml or .json extension).
--statement-defaults string JSON string defining the environment's Flink statement defaults, or path to a file to read defaults from (with .yml, .yaml or .json extension). Expected shape: {"detached":{"flinkConfiguration":{...}},"interactive":{"flinkConfiguration":{...}}}.
--compute-pool-defaults string JSON string defining the environment's Flink compute pool defaults, or path to a file to read defaults from (with .yml, .yaml or .json extension).
--url string Base URL of the Confluent Manager for Apache Flink (CMF). Environment variable "CONFLUENT_CMF_URL" may be set in place of this flag.
--client-key-path string Path to client private key for mTLS authentication. Environment variable "CONFLUENT_CMF_CLIENT_KEY_PATH" may be set in place of this flag.
Expand Down
2 changes: 1 addition & 1 deletion test/fixtures/output/flink/environment/create-no-namespace.golden
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ Usage:
Flags:
--kubernetes-namespace string REQUIRED: Kubernetes namespace to deploy Flink applications to.
--defaults string JSON string defining the environment's Flink application defaults, or path to a file to read defaults from (with .yml, .yaml or .json extension).
--statement-defaults string JSON string defining the environment's Flink statement defaults, or path to a file to read defaults from (with .yml, .yaml or .json extension).
--statement-defaults string JSON string defining the environment's Flink statement defaults, or path to a file to read defaults from (with .yml, .yaml or .json extension). Expected shape: {"detached":{"flinkConfiguration":{...}},"interactive":{"flinkConfiguration":{...}}}.
--compute-pool-defaults string JSON string defining the environment's Flink compute pool defaults, or path to a file to read defaults from (with .yml, .yaml or .json extension).
--url string Base URL of the Confluent Manager for Apache Flink (CMF). Environment variable "CONFLUENT_CMF_URL" may be set in place of this flag.
--client-key-path string Path to client private key for mTLS authentication. Environment variable "CONFLUENT_CMF_CLIENT_KEY_PATH" may be set in place of this flag.
Expand Down
1 change: 1 addition & 0 deletions test/fixtures/output/flink/environment/create-statement-defaults-invalid.golden
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Error: failed to parse statement defaults: json: unknown field "config-overrides"
2 changes: 1 addition & 1 deletion test/fixtures/output/flink/environment/missing-flag-failure.golden
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ Flags:
--client-cert-path string Path to client cert to be verified by Confluent Manager for Apache Flink. Include for mTLS authentication. Environment variable "CONFLUENT_CMF_CLIENT_CERT_PATH" may be set in place of this flag.
--certificate-authority-path string Path to a PEM-encoded Certificate Authority to verify the Confluent Manager for Apache Flink connection. Environment variable "CONFLUENT_CMF_CERTIFICATE_AUTHORITY_PATH" may be set in place of this flag.
--defaults string JSON string defining the environment's Flink application defaults, or path to a file to read defaults from (with .yml, .yaml or .json extension).
--statement-defaults string JSON string defining the environment's Flink statement defaults, or path to a file to read defaults from (with .yml, .yaml or .json extension).
--statement-defaults string JSON string defining the environment's Flink statement defaults, or path to a file to read defaults from (with .yml, .yaml or .json extension). Expected shape: {"detached":{"flinkConfiguration":{...}},"interactive":{"flinkConfiguration":{...}}}.
--compute-pool-defaults string JSON string defining the environment's Flink compute pool defaults, or path to a file to read defaults from (with .yml, .yaml or .json extension).
-o, --output string Specify the output format as "human", "json", or "yaml". (default "human")

Expand Down
2 changes: 1 addition & 1 deletion test/fixtures/output/flink/environment/update-help-onprem.golden
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ Flags:
--client-cert-path string Path to client cert to be verified by Confluent Manager for Apache Flink. Include for mTLS authentication. Environment variable "CONFLUENT_CMF_CLIENT_CERT_PATH" may be set in place of this flag.
--certificate-authority-path string Path to a PEM-encoded Certificate Authority to verify the Confluent Manager for Apache Flink connection. Environment variable "CONFLUENT_CMF_CERTIFICATE_AUTHORITY_PATH" may be set in place of this flag.
--defaults string JSON string defining the environment's Flink application defaults, or path to a file to read defaults from (with .yml, .yaml or .json extension).
--statement-defaults string JSON string defining the environment's Flink statement defaults, or path to a file to read defaults from (with .yml, .yaml or .json extension).
--statement-defaults string JSON string defining the environment's Flink statement defaults, or path to a file to read defaults from (with .yml, .yaml or .json extension). Expected shape: {"detached":{"flinkConfiguration":{...}},"interactive":{"flinkConfiguration":{...}}}.
--compute-pool-defaults string JSON string defining the environment's Flink compute pool defaults, or path to a file to read defaults from (with .yml, .yaml or .json extension).
-o, --output string Specify the output format as "human", "json", or "yaml". (default "human")

Expand Down
1 change: 1 addition & 0 deletions test/fixtures/output/flink/environment/update-statement-defaults-invalid.golden
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Error: failed to parse statement defaults: json: unknown field "config-overrides"
2 changes: 2 additions & 0 deletions test/flink_onprem_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,7 @@ func (s *CLITestSuite) TestFlinkEnvironmentCreate() {
{args: "flink environment create default-failure --kubernetes-namespace default-staging", fixture: "flink/environment/create-failure.golden", exitCode: 1},
{args: "flink environment create default --kubernetes-namespace default-staging", fixture: "flink/environment/create-existing.golden", exitCode: 1},
{args: "flink environment create default", fixture: "flink/environment/create-no-namespace.golden", exitCode: 1},
{args: "flink environment create default-2 --kubernetes-namespace default-staging --statement-defaults '{\"config-overrides\":{\"key\":\"value\"}}'", fixture: "flink/environment/create-statement-defaults-invalid.golden", exitCode: 1},
// success with application, statement and compute pool defaults
{args: "flink environment create default-2" +
" --defaults test/fixtures/input/flink/environment/application-defaults.json" +
Expand All @@ -288,6 +289,7 @@ func (s *CLITestSuite) TestFlinkEnvironmentUpdate() {
{args: "flink environment update non-existent --defaults '{\"property\": \"value\"}'", fixture: "flink/environment/update-non-existent.golden", exitCode: 1},
{args: "flink environment update get-failure --defaults '{\"property\": \"value\"}'", fixture: "flink/environment/update-get-failure.golden", exitCode: 1},
{args: "flink environment update missing-flag-failure", fixture: "flink/environment/missing-flag-failure.golden", exitCode: 1},
{args: "flink environment update default --statement-defaults '{\"config-overrides\":{\"key\":\"value\"}}'", fixture: "flink/environment/update-statement-defaults-invalid.golden", exitCode: 1},
// success with application, statement and compute pool defaults
{args: "flink environment update default" +
" --defaults test/fixtures/input/flink/environment/application-defaults.json" +
Expand Down