Skip to content
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.

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

cli: add field docs to the state file #2453

Merged
merged 4 commits into from
Oct 16, 2023
Merged

Conversation

msanft
Copy link
Contributor

@msanft msanft commented Oct 16, 2023

Context

The state file was missing in-line documentation, aligned with what the config file has.

Proposed change(s)

  • Add in-line documentation to the fields in the state file.
  • Mark the fields that (always) need to be filled manually in case of self-managed infrastructure.

Additional info

Checklist

  • Add labels (e.g., for changelog category)
  • Is PR title adequate for changelog?
  • Link to Milestone

@msanft msanft added the feature This introduces new functionality label Oct 16, 2023
@msanft msanft added this to the v2.13.0 milestone Oct 16, 2023
@msanft msanft requested a review from elchead as a code owner October 16, 2023 07:27
@netlify
Copy link

netlify bot commented Oct 16, 2023

Deploy Preview for constellation-docs canceled.

Name Link
🔨 Latest commit d0de973
🔍 Latest deploy log https://app.netlify.com/sites/constellation-docs/deploys/652d4406b4e58c00083b2e0b

// description: |
// DO NOT EDIT. State of the Constellation Kubernetes cluster.
// These values are set during cluster initialization and should not be changed.
ClusterValues ClusterValues `yaml:"clusterValues"`
Copy link
Contributor

@elchead elchead Oct 16, 2023

Choose a reason for hiding this comment

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

future idea: we could back up these values in the cluster and compare against the cluster state upon apply, similar to the attestationconfig

Copy link
Member

@daniel-weisse daniel-weisse left a comment

Choose a reason for hiding this comment

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

We should rethink custom marshaling of []byte for the state file.
Currently, MeasurementSalt and InitSecret get rendered as a list of byte values, e.g.

MeasurementSalt:
    - 12
    - 53
    - 12
    - 53
    // ...

Should probably be done in a separate PR however.

Otherwise looks good to me.

@msanft msanft force-pushed the feat/cli/document-state-file branch from 0df5aa9 to 47300cf Compare October 16, 2023 08:11
@msanft msanft force-pushed the feat/cli/document-state-file branch from 180e342 to 1d9b72c Compare October 16, 2023 13:05
@github-actions
Copy link
Contributor

Coverage report

Package Old New Trend
cli/internal/state 60.00% 95.50% ↗️

@msanft msanft merged commit e5513f1 into main Oct 16, 2023
8 checks passed
@msanft msanft deleted the feat/cli/document-state-file branch October 16, 2023 14:49
elchead pushed a commit that referenced this pull request Oct 17, 2023
* add field docs to the state file

* mark only optional fields

* tidy

* use talos encoder
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature This introduces new functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants