Skip to content

Conversation

partcyborg
Copy link
Contributor

Replace StringSliceVarP with StringArrayVarP for the --include and --exclude flags so regex strings can contain colons.

With the current StringSliceVarP implementation, it is impossible to accurately search for (or eliminate) a specific key/value pair if your service emits log lines in json:

$ stern -e '"pod.iam.role":""' r1-kiam-agent
Error: invalid argument "\"pod.iam.role\":\"\"" for "-e, --exclude" flag: parse error on line 1, column 14: extraneous or missing " in quoted-field
Usage:
  stern pod-query [flags]

With these flags defined as StringArrayVarP this works correctly:

$ dist/stern -e '"pod.iam.role":""' r1-kiam-agent
+ r1-kiam-agent-kg85t › kiam-agent

Replace `StringSliceVarP` with `StringArrayVarP` for the --include and
--exclude flags so regex strings can contain colons.
Copy link
Member

@rkmathi rkmathi left a comment

Choose a reason for hiding this comment

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

Thank you for your pull request!
I think this change is good.
To pass the test, could you please respond to the README as it needs to be changed?

./hack/verify-readme.sh
--- ./README.md	2022-10-16 01:37:3[5](https://github.com/stern/stern/actions/runs/3246025703/jobs/5349447581#step:6:6).[6](https://github.com/stern/stern/actions/runs/3246025703/jobs/5349447581#step:6:7)882[7](https://github.com/stern/stern/actions/runs/3246025703/jobs/5349447581#step:6:8)6651 +0000
+++ /tmp/tmp.decxYMhPJw	2022-10-16 01:3[8](https://github.com/stern/stern/actions/runs/3246025703/jobs/5349447581#step:6:9):36.[9](https://github.com/stern/stern/actions/runs/3246025703/jobs/5349447581#step:6:10)57049283 +0000
@@ -70,[11](https://github.com/stern/stern/actions/runs/3246025703/jobs/5349447581#step:6:12) +70,11 @@
  `--container-state`         | `running` | Tail containers with state in running, waiting or terminated. To specify multiple states, repeat this or set comma-separated value.
  `--context`                 |           | Kubernetes context to use. Default to current context configured in kubeconfig.
  `--ephemeral-containers`    | `true`    | Include or exclude ephemeral containers.
- `--exclude`, `-e`           |           | Log lines to exclude. (regular expression)
+ `--exclude`, `-e`           | `[]`      | Log lines to exclude. (regular expression)
  `--exclude-container`, `-E` |           | Container name to exclude when multiple containers in pod. (regular expression)
  `--exclude-pod`             |           | Pod name to exclude. (regular expression)
  `--field-selector`          |           | Selector (field query) to filter on. If present, default to ".*" for the pod-query.
- `--include`, `-i`           |           | Log lines to include. (regular expression)
+ `--include`, `-i`           | `[]`      | Log lines to include. (regular expression)
  `--init-containers`         | `true`    | Include or exclude init containers.
  `--kubeconfig`              |           | Path to kubeconfig file to use. Default to KUBECONFIG variable then ~/.kube/config path.
  `--namespace`, `-n`         |           | Kubernetes namespace to use. Default to namespace configured in kubernetes context. To specify multiple namespaces, repeat this or set comma-separated value.

Copy link

@jackromo888 jackromo888 left a comment

Choose a reason for hiding this comment

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

@superbrothers
Copy link
Member

@rkmathi I've updated README.md. PTAL.

@rkmathi rkmathi self-requested a review December 24, 2022 15:19
Copy link
Member

@rkmathi rkmathi left a comment

Choose a reason for hiding this comment

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

@superbrothers Thank you for fixing!

@superbrothers superbrothers merged commit 80a68a9 into stern:master Dec 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants