Ett 241 json2yaml format #12

liseli · 2025-11-10T18:58:39Z

This PR implements the logic to generate secrets in YAML format. Jira ticket: https://hathitrust.atlassian.net/browse/ETT-241

The secrets are generated in JSON format from the cluster, and then they are converted to YAML when the user adds --format yaml to the commands diff, pull, and seal.

The implementation here is used to serialize and deserialize all secrets; this is the strategy for managing secrets with multiple lines.

Example of how to use each command

Pull secrets as YAML
tkseal pull /path/to/env --format yaml
Seal secrets to YAML
tkseal seal /path/to/env --format yaml
Diff with YAML files
tkseal diff /path/to/env --format yaml

Copilot

Pull Request Overview

This PR adds YAML format support to the tkseal tool, allowing users to manage secrets in either JSON or YAML format. The implementation introduces a --format flag to the diff, pull, and seal commands, defaulting to JSON for backward compatibility.

Key changes:

New serialization module with YAML/JSON conversion functions and custom YAML representers for multiline strings
Secret type filtering system with allowed, carefully-managed, and forbidden secret types
Format-aware file path generation and content normalization for consistent comparisons
Forbidden secrets warning feature in the pull command

Reviewed Changes

Copilot reviewed 25 out of 28 changed files in this pull request and generated 13 comments.

Show a summary per file

File	Description
`src/tkseal/serializers.py`	New module for JSON/YAML serialization with multiline string handling
`src/tkseal/configuration.py`	Added secret type constants (MANAGED, FORBIDDEN) for filtering
`src/tkseal/secret_state.py`	Added format parameter and forbidden secrets caching mechanism
`src/tkseal/secret.py`	Implemented secret type filtering and forbidden secret tracking
`src/tkseal/seal.py`	Updated to support both JSON and YAML output formats
`src/tkseal/pull.py`	Added format conversion when writing secrets
`src/tkseal/diff.py`	Added normalization to JSON for format-agnostic comparison
`src/tkseal/tkseal_utils.py`	Added normalize_to_json utility function and improved docstrings
`src/tkseal/cli.py`	Added --format option to diff, pull, and seal commands; added forbidden secrets warnings
`tests/test_serializers.py`	New test file covering serialization round-trips and multiline handling
`tests/test_secret.py`	Added tests for secret type filtering and forbidden secret tracking
`tests/test_seal.py`	Parameterized tests for both JSON and YAML formats
`tests/test_pull.py`	Parameterized tests for both JSON and YAML formats
`tests/test_cli.py`	Added format flag tests and forbidden secrets warning tests
`tests/conftest.py`	Updated fixtures with type field and format support
`README.md`	Added documentation for forbidden secrets feature and format flag usage
`poetry.lock`	Updated coverage dependency from 7.11.0 to 7.11.1
`pyproject.toml`	Added py.typed file to package for mypy support

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-10T20:52:36Z

README.md

 # Type checking
 poetry run mypy src/
+
+Note: The file `py.typed`  has added to the package and specified in pyproject.toml to ensure mypy treats 


Grammar error: "has added" should be "has been added".

Suggested change

Note: The file `py.typed` has added to the package and specified in pyproject.toml to ensure mypy treats

Note: The file `py.typed` has been added to the package and specified in pyproject.toml to ensure mypy treats

Copilot · 2025-11-10T20:52:36Z

src/tkseal/serializers.py

+    elif format == "yaml":
+        return yaml.dump(
+            data,
+            default_flow_style=False, # Controls the output style. False means indented block forma. with each item on a new line. Preferred output for config files.


Typo in comment: "forma." should be "format."

Suggested change

default_flow_style=False, # Controls the output style. False means indented block forma. with each item on a new line. Preferred output for config files.

default_flow_style=False, # Controls the output style. False means indented block format with each item on a new line. Preferred output for config files.

Copilot · 2025-11-10T20:52:37Z

src/tkseal/cli.py

+        # click.secho(
+        #    f'This shows what would change in the cluster based on "plain_secrets.{format}"',
+        #    fg="yellow",
+        # )

        # Show diff to preview changes
-        diff_obj = Diff(secret_state)
-        result = diff_obj.plain()
+        # diff_obj = Diff(secret_state)
+        # result = diff_obj.plain()

        # Display diff results
-        if result.has_differences:
-            click.echo(result.diff_output)
-
-            # Confirm before sealing
-            if click.confirm("Are you sure?"):
-                seal_obj = Seal(secret_state)
-                seal_obj.run()
-                click.echo("Successfully sealed secrets to sealed_secrets.json")
-        else:
-            click.echo("No differences")
+        # if result.has_differences:
+        #    click.echo(result.diff_output)
+
+        # Confirm before sealing
+        if click.confirm("Are you sure?"):
+            seal_obj = Seal(secret_state)
+            seal_obj.run()
+            click.echo(f"Successfully sealed secrets to {sealed_secrets_file}")
+        # else:
+        #    click.echo("No differences")


Large blocks of commented-out code should be removed. If this functionality might be needed later, consider using version control history instead. The commented code removes the diff preview before sealing, which seems like it might be an intentional behavior change that should either be implemented properly or documented in the PR description.

Copilot · 2025-11-10T20:52:37Z

src/tkseal/secret_state.py

+        assert self._secrets_cache is not None
+        return self._secrets_cache.to_json()
+
+    def get_forbidden_secrets(self) -> Any:


The return type annotation Any is too generic. The docstring specifies the return type as list[dict[str, str]], which should be used as the actual type annotation instead of Any. Using Any defeats the purpose of type hints and reduces type safety.

Copilot · 2025-11-10T20:52:37Z

src/tkseal/secret.py

+        Args:
+            raw_secrets: kubectl output dict with 'items' key
+        Returns:
+            List of secrets excluding forbidden types


The docstring says "Filter out forbidden secret types" but the return description says "List of secrets excluding forbidden types", which is misleading. The function actually returns a list OF the forbidden secrets (not excluding them). The docstring should say "Returns: List of forbidden secrets found in raw_secrets".

Suggested change

List of secrets excluding forbidden types

List of forbidden secrets found in raw_secrets

Copilot · 2025-11-10T20:52:38Z

src/tkseal/secret_state.py


    @property
-    def namespace(self) -> str:
+    def namespace(self) -> Any:


The return type annotation Any is too generic. Based on the docstring and the actual return value (self._tk_env.namespace), this should return str. Using Any defeats the purpose of type hints and reduces type safety.

Suggested change

def namespace(self) -> Any:

def namespace(self) -> str:

Copilot · 2025-11-10T20:52:39Z

src/tkseal/secret_state.py

        return self._tk_env.namespace

-    def plain_secrets(self) -> str:
+    def plain_secrets(self) -> Any:


The return type annotation Any is too generic. Based on the docstring and the actual return value (file content as string), this should return str. Using Any defeats the purpose of type hints and reduces type safety.

Suggested change

def plain_secrets(self) -> Any:

def plain_secrets(self) -> str:

Copilot · 2025-11-10T20:52:39Z

src/tkseal/secret_state.py

            return ""

-    def kube_secrets(self) -> str:
+    def kube_secrets(self) -> Any:


The return type annotation Any is too generic. Based on the docstring and the method implementation which returns self._secrets_cache.to_json(), this should return str. Using Any defeats the purpose of type hints and reduces type safety.

Suggested change

def kube_secrets(self) -> Any:

def kube_secrets(self) -> str:

Copilot · 2025-11-10T20:52:39Z

src/tkseal/secret.py

+    @staticmethod
+    def filter_out_forbidden_secrets(
+        raw_secrets: dict[str, Any],
+    ) -> list[dict[str, Any]]:


The return type list[dict[str, Any]] is too generic. Based on the implementation (line 120: {secret.name: secret.type}) where both are strings, the return type should be list[dict[str, str]] for better type safety.

Suggested change

) -> list[dict[str, Any]]:

) -> list[dict[str, str]]:

Copilot · 2025-11-10T20:52:39Z

src/tkseal/cli.py

+            seal_obj.run()
+            click.echo(f"Successfully sealed secrets to {sealed_secrets_file}")
+        # else:
+        #    click.echo("No differences")


This comment appears to contain commented-out code.

Suggested change

# click.echo("No differences")

aelkiss

A few things:

It looks like this PR also includes the commits from #11 - probably we should change this to draft, get #11 to the point where we can merge it, then rebase this.
The changes to make the filename / format more generic rather than hard coding plain_secrets.json make sense to me.
JSON vs. YAML serialization seems like a good case for having two different classes with the same interface, rather than the procedural interface with conditionals as we have here
I think the interface (--format yaml) is probably fine for the time being. I was wondering a bit if we should always specify the filename and infer the format (but that maybe risks hiding the functionality of different formats), or for tkseal seal use the YAML file if it exists and the JSON doesn't (but then what do we do if both exist). So I think this is fine for now and we can adjust it later if need be.

aelkiss · 2025-11-10T21:11:00Z

src/tkseal/serializers.py

Rather than having conditionals here this seems like a good case for having separate JSONSerializer and YAMLSerializer objects that implement the same interface.

I've carefully reviewed and incorporated your feedback to enhance this PR. I've also created separate classes for serializing secrets. PR11 has already been merged into main, and I've resolved the conflicts on this PR, so it's now ready to be merged.

aelkiss · 2025-11-10T21:12:01Z

src/tkseal/serializers.py

+import yaml
+
+
+def _str_presenter(dumper, data):


Should cite where I got this (https://www.hrekov.com/blog/yaml-formatting-custom-representer)

… the new feature branch with main branch

…zer and YAMLSerializer

liseli requested review from Ronster2018, aelkiss and Copilot and removed request for Ronster2018 November 10, 2025 20:45

Copilot started reviewing on behalf of liseli November 10, 2025 20:45 View session

Copilot finished reviewing on behalf of liseli November 10, 2025 20:51

Copilot AI reviewed Nov 10, 2025

View reviewed changes

aelkiss reviewed Nov 10, 2025

View reviewed changes

liseli force-pushed the ETT-241_json2yaml_format branch from 859a975 to 8650023 Compare November 13, 2025 17:46

Update tkseal application to generate secrets in YAML format. Merging…

a02b73c

… the new feature branch with main branch

liseli force-pushed the ETT-241_json2yaml_format branch from 8650023 to a02b73c Compare November 13, 2025 19:44

Ronster2018 approved these changes Nov 14, 2025

View reviewed changes

Implementing secrets serializer as independents classes - JSONSeriali…

17d48c3

…zer and YAMLSerializer

liseli merged commit cd2818a into main Nov 17, 2025
1 check passed

liseli deleted the ETT-241_json2yaml_format branch November 17, 2025 21:42

	Note: The file `py.typed` has added to the package and specified in pyproject.toml to ensure mypy treats
	Note: The file `py.typed` has been added to the package and specified in pyproject.toml to ensure mypy treats

	default_flow_style=False, # Controls the output style. False means indented block forma. with each item on a new line. Preferred output for config files.
	default_flow_style=False, # Controls the output style. False means indented block format with each item on a new line. Preferred output for config files.

	List of secrets excluding forbidden types
	List of forbidden secrets found in raw_secrets

	def plain_secrets(self) -> Any:
	def plain_secrets(self) -> str:

	def kube_secrets(self) -> Any:
	def kube_secrets(self) -> str:

Ett 241 json2yaml format #12

Ett 241 json2yaml format #12

Uh oh!

Conversation

liseli commented Nov 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

aelkiss left a comment

Choose a reason for hiding this comment

Uh oh!

aelkiss Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

liseli Nov 17, 2025

Choose a reason for hiding this comment

Uh oh!

aelkiss Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

liseli commented Nov 10, 2025 •

edited

Loading