tidy

hips/hip-00XX.md

@@ -10,11 +10,11 @@ status: "draft"
 ## Abstract
 
 <!-- A short (~200 word) description of the technical issue being addressed. -->
-Helm 3 introduced the [three-way strategic merge and patch](https://helm.sh/docs/faq/changes_since_helm2/#improved-upgrade-strategy-3-way-strategic-merge-patches) methology, in order to allow Helm to update resources which may have been modified by other processes.
+Helm 3 introduced the [three-way strategic merge and patch](https://helm.sh/docs/faq/changes_since_helm2/#improved-upgrade-strategy-3-way-strategic-merge-patches) methology. Allowing Helm to update resources which may have been modified by other processes.
 
-Kubernetes now offers [Server-Side Apply](https://kubernetes.io/docs/reference/using-api/server-side-apply/)(SSA)(GA since v1.22), that has serveral advantages over these client-side apply (CSA) methods that Helm, and e.g. Kubectl traditionally incorporated.
+Kubernetes now offers [Server-Side Apply](https://kubernetes.io/docs/reference/using-api/server-side-apply/) (SSA)(GA since v1.22), that has serveral advantages over the client-side apply (CSA) methods that Helm, and e.g. Kubectl traditionally incorporated.
 
-This HIP proposes Helm to utilize Kubernete's Server-Side Apply for Helm resource management, and how Helm can migrate to doing so. But limits scope to incorporating SSA as an opt-in feature only. And defers removing / changing existing usaging of the (client-side) three-way strategic merge patch processes (without opt-in).
+This HIP proposes enabling Helm to utilize Kubernete's Server-Side Apply for Helm resource management. But limits scope to incorporating SSA as an opt-in feature only. And defers removing / changing existing usaging of the (client-side) three-way strategic merge patch processes (without opt-in).
 
 ## Motivation
 
@@ -25,36 +25,21 @@ The primary motivation for Helm to switch to Server-Side Apply is to gain the be
 
 https://kubernetes.io/blog/2022/10/20/advanced-server-side-apply/
 
-Helm supporting SSA will allow users to realize the benefits of SSA. And longer term goals (out of scope for this HIP) might be for Helm to utilize SSA by default ((similar to kubectl)[https://github.com/kubernetes/enhancements/tree/master/keps/sig-cli/3805-ssa-default>]), and as a code mantainence benefit, eventually drop support for CSA/atrategic-merge patch process/code.
+Helm supporting SSA will allow users to realize the benefits of SSA. And longer term goals (out of scope for this HIP) might be for Helm to utilize SSA by default ([similar to kubectl](https://github.com/kubernetes/enhancements/tree/master/keps/sig-cli/3805-ssa-default>)), and later as a code mantainence benefit, eventually drop support for the strategic-merge patch client-side apply process/code.
 
 ## Rationale
 
 <!-- Describe why particular design decisions were made. -->
 
-Due to the backwards compatibility concerns documented below, the decision was made to provide opt-in support only. And even when a chart is deployed with SSA enabled, Helm should mantain full compatibility with reverting back to CSA. Additionally it was desirable to mimic kubectl's cli interface/flags, as a way for Helm to be consistent with an existing kubectl functionality.
-
- <!--
- Presumably in the future it will be desired to make SSA the default (e.g. Helm 4), and eventually remove support for client-side apply (CSA). But due to older Helm clients not supporting SSA, and the unclear timeline and success of using SSA, it is desired to defer those decisions to another date/HIP.
--->
-
-<!--
-Kubernetes mantainers for kubectl:
-
-https://github.com/kubernetes/enhancements/tree/master/keps/sig-cli/3805-ssa-default
-
-Secondary motivations include:
-- Reducing the complexity of Helm: namely paving the way to remove the need for Helm to support strategic merge patch logic
-- Removing the need for Helm to store "current"  resource manifests (presumably greatly reducing the size and scope of the release manifest)
--->
-
+Due to the backwards compatibility concerns documented below, the decision was made to provide opt-in support only. And even when a chart is deployed with SSA enabled, Helm should mantain full compatibility with reverting back to CSA. Additionally it was desirable to mimic kubectl's cli interface/flags. As a way for Helm to be consistent with an existing kubectl functionality.
 
 ## Specification
 
 <!-- Describe the syntax and semantics of any new feature. -->
 
 ### General
 
-Helm will add a new flag to the install, upgrade and rollback sub-commands: `--server-side=false|true|auto`, modelled after the like `kubectl` flag. And a corresponding field to the respective install/upgrade/rollback SDK API objects. Helm will default to `false` for installs, and retain its existing behaviour. When `true`, Helm will submit resources to the Kubernetes API server with SSA enabled. When `auto`, Helm will default to CSA on installs (`false`), and for upgrades and rollbacks enable SSA based on the whether SSA was utilized for the prior operation (`auto`). The usage of SSA for a given release will be stored in the release's manifest.
+Helm will add a new flag to the install, upgrade and rollback sub-commands: `--server-side=false|true|auto`, modelled after the like `kubectl` flag. And add a corresponding field to the respective install/upgrade/rollback SDK API objects. Helm will default to `false` for installs, retaining its existing behaviour. When `true`, Helm will submit resources to the Kubernetes API server with SSA enabled. When `auto`, Helm will default to CSA on installs (`false`), and for upgrades and rollbacks enable SSA based on the whether SSA was utilized for the prior operation (`auto`). The usage of SSA for a given release will be stored in the release's manifest.
 
 If the user attempts to use SSA with a cluster which does not support it (unlikely: SSA [went GA](https://kubernetes.io/blog/2021/08/06/server-side-apply-ga/) in Kubernetes 1.22), Helm should error.
 
@@ -70,10 +55,9 @@ Helm will utilize SSA for custom resources, which previously it replaced (TODO:
 
 It is possible when Helm upgrades a chart, that there will be a field conflict with another field manager. In this case, Helm should report the error to the user.
 
-Helm will also add a new flag `--force-conflicts`, which will tell the apply to ignore/override field conflicts (and so thus make Helm the field manager).
-
-Of note, Helm upgrade already includes the `--force` flag, which tells Helm to replace objects <TODO: confirm this, going from memory>. However, given this different semantics, it is thought a new flag is required.
+Helm will also add a new flag `--force-conflicts`, and corresponding field to the `upgrade` and `rollback` commands. Which will tell the apply to ignore/override field conflicts (and so thus make Helm the field manager).
 
+(Of note, Helm upgrade already includes the `--force` flag, which tells Helm to replace objects <TODO: confirm this, going from memory>. However, given this different semantics, it is thought a new flag is required)
 
 ### Implementation
 
@@ -85,48 +69,43 @@ https://github.com/kubernetes/kubectl/blob/197123726db24c61aa0f78d1f0ba6e91a2ec2
 
 #### Special
 
-<TODO: dry-run client>: --dry-run=client doesn't work with --server-side
+- `--dry-run=client` doesn't work with SSA enabled
+
 
 ## Backwards compatibility
 
 <!-- Describe potential impact and severity on pre-existing code. -->
 
-Server-Side Apply
-FEATURE STATE: Kubernetes v1.22 [stable]
-
-https://github.com/kubernetes/enhancements/tree/master/keps/sig-cli/3805-ssa-default#summary
-https://github.com/kubernetes/community/blob/master/contributors/devel/sig-api-machinery/strategic-merge-patch.md
-
 There are thought to be three main compatibility concerns for Helm switching to SSA:
 1. The differences between resource manifests managed with SSA vs the existing three-way strategic merge process
 2. Issues with a user enabling/disabling SSA over the lifetime of a release e.g. environment differences or older Helm clients which don't understand SSA
 3. Requirement of PATCH <TODO: check if this is new>
 
 For 1., this HIP limits Helm to opt-in to server side apply, allowing users to verify SSA's behaviour matches their expectations. For 2., the user can mitigate by ensuring SSA is consistantly used within their environment. But the expectation is that SSA should generally produce equivalient Kubernetes objects as CSA.
 
+Additionally as noted, SSA isn't compatible with very old Kubernetes versions (SSA beta Kubernetes 1.18)
+
 
 ## Security implications
 
 <!-- How could a malicious user take advantage of this new feature? -->
 
-There should not be any additional security concerns. As the method of updating object manifests continues to follow the existing RBAC requirements as client-side apply (TODO: read through SSA docs to confirm this)
-
-TODO: With exception of requiring additional PATCH access
-
+There should not be any additional security concerns.
 
 ## How to teach this
 
 <!-- How to teach users, new and experienced, how to apply the HIP to their work. -->
 
-Documentation for how Helm does resource upgrades, and the advantages of SSA. Also documentation for the `--server-side=false|true|auto` flag, and the defaults for install/upgrades. And the corresponding documentation for the field(s) SDK API objects.
+Documentation for how Helm does resource upgrades, and the advantages of SSA. Also documentation for the `--server-side=false|true|auto` and `--force-conflicts=false|true` flags, and the defaults for install/upgrades/rollbacks. And the corresponding documentation for the field(s) SDK API objects.
 
 ## Reference implementation
 
 <!-- Link to any existing implementation and details about its state, e.g.
 proof-of-concept. -->
 
-https://github.com/kubernetes/kubectl/blob/197123726db24c61aa0f78d1f0ba6e91a2ec2f35/pkg/cmd/apply/apply.go#L248
-https://github.com/kubernetes/kubectl/blob/197123726db24c61aa0f78d1f0ba6e91a2ec2f35/pkg/cmd/apply/apply.go#L564-L657
+Kubectl:
+<https://github.com/kubernetes/kubectl/blob/197123726db24c61aa0f78d1f0ba6e91a2ec2f35/pkg/cmd/apply/apply.go#L248>
+<https://github.com/kubernetes/kubectl/blob/197123726db24c61aa0f78d1f0ba6e91a2ec2f35/pkg/cmd/apply/apply.go#L564-L657>
 
 
 ## Rejected ideas