Skip to content

Commit

Permalink
compute version field in Check, use 3-way diff (#2672)
Browse files Browse the repository at this point in the history 
<!--Thanks for your contribution. See [CONTRIBUTING](CONTRIBUTING.md)
    for Pulumi's contribution guidelines.

    Help us merge your changes more quickly by adding more details such
    as labels, milestones, and reviewers.-->

### Proposed changes

<!--Give us a brief description of what you've done and what it solves.
-->
This PR stabilizes Helm upgrades **by abandoning the use of checksums**
(which was added in v4.5.0, see
[PR](#2568)), while
allowing for local chart change detection via the chart's version
number. That is, the user would be expected to touch the chart's version
to compel Pulumi to roll out the change.

Implementation-wise, this is accomplished by resolving the `version`
property during `Check`. The checked input reflects the goal version,
and if the goal has changed, then `Diff` detects this in the natural way
and effects an upgrade. `Check` no longer uses `helm template` to render
the chart, it simply resolves the chart version.

The diff logic is improved here to use a three-way merge, to be able to
detect drift in the actual Helm release.

The refresh logic is improved to not mutate inputs (as was done in the
past to help with drift detection).

### Related issues (optional)

Closes #2649

<!--Refer to related PRs or issues: #1234, or 'Fixes #1234' or 'Closes
#1234'.
Or link to full URLs to issues or pull requests in other GitHub
repositories. -->
  • Loading branch information
@EronWright
EronWright authored Nov 27, 2023
1 parent 9078fd3 commit d90d153
Show file tree
Hide file tree
Showing 10 changed files with 198 additions and 642 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@

- Fix: Make the invoke calls for Helm charts and YAML config resilient to the value being None or an empty dict (https://github.com/pulumi/pulumi-kubernetes/pull/2665)

- Fix: compute version field in Check for content detection (https://github.com/pulumi/pulumi-kubernetes/pull/2672)

## 4.5.4 (November 8, 2023)
- Fix: Helm Release: chart requires kubeVersion (https://github.com/pulumi/pulumi-kubernetes/pull/2653)

Expand Down
2 changes: 1 addition & 1 deletion provider/cmd/pulumi-gen-kubernetes/main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -711,7 +711,7 @@ func buildPulumiFieldsFromTerraform(path string, block *TerraformBlockSchema) ma
if path == "kubernetes_deployment.spec.template.spec.container" && blockName == "port" {
field["name"] = "ports"
}
// 3. kubernetes_service has a field "port" wich is a list, but we call it "ports"
// 3. kubernetes_service has a field "port" which is a list, but we call it "ports"
if path == "kubernetes_service.spec" && blockName == "port" {
field["name"] = "ports"
}
Expand Down
191 changes: 65 additions & 126 deletions provider/pkg/provider/helm_release.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@ package provider

import (
"context"
"crypto/sha256"
"encoding/hex"
"encoding/json"
"errors"
"fmt"
Expand All @@ -29,7 +27,6 @@ import (
"strings"
"time"

jsonpatch "github.com/evanphx/json-patch"
pbempty "github.com/golang/protobuf/ptypes/empty"
"github.com/imdario/mergo"
"github.com/mitchellh/mapstructure"
Expand All @@ -55,6 +52,7 @@ import (
"helm.sh/helm/v3/pkg/release"
"helm.sh/helm/v3/pkg/repo"
"helm.sh/helm/v3/pkg/storage/driver"
"k8s.io/apimachinery/pkg/util/strategicpatch"
"k8s.io/client-go/rest"
"k8s.io/client-go/tools/clientcmd"
"k8s.io/client-go/tools/clientcmd/api"
Expand Down Expand Up @@ -134,8 +132,6 @@ type Release struct {
// Manifest map[string]interface{} `json:"manifest,omitempty"`
// Names of resources created by the release grouped by "kind/version".
ResourceNames map[string][]string `json:"resourceNames,omitempty"`
// The checksum of the rendered template (to detect changes)
Checksum string `json:"checksum,omitempty"`
// Status of the deployed release.
Status *ReleaseStatus `json:"status,omitempty"`
}
Expand Down Expand Up @@ -361,19 +357,27 @@ func (r *helmReleaseProvider) Check(ctx context.Context, req *pulumirpc.CheckReq
return nil, err
}

err = r.setComputedInputs(ctx, urn, new)
logger.V(9).Infof("Loading Helm chart.")
chart, err := r.helmLoad(ctx, urn, new)
if err != nil {
// setComputedInputs fails when the chart cannot be rendered, and we report it as a problem with the `chart` input.
failures = append(failures, &pulumirpc.CheckFailure{
Property: "chart",
Reason: fmt.Sprintf("%v; check the chart name and repository configuration.", err),
})
} else {
// determine the desired state of the resource, i.e the specific chart version
// as opposed to the program input (which is a constraint such as ">= 1.2.3").
// with this we may determine whether the Helm release needs to be upgraded.
new.Version = chart.Metadata.Version
}

logger.V(9).Infof("New: %+v", new)
news = resource.NewPropertyMap(new)
}

// remove deprecated inputs
delete(news, "resourceNames")

newInputs, err := plugin.UnmarshalProperties(newResInputs, plugin.MarshalOptions{
Label: fmt.Sprintf("%s.newInputs", label),
KeepUnknowns: true,
Expand Down Expand Up @@ -425,47 +429,21 @@ func (r *helmReleaseProvider) setDefaults(target resource.PropertyMap) {
}
}

// setComputedInputs augments resource inputs with computed inputs.
func (r *helmReleaseProvider) setComputedInputs(ctx context.Context, urn resource.URN, new *Release) error {

// generate the template output
manifest, err := r.helmTemplate(ctx, urn, new)
if err != nil {
return err
}

// compute the checksum of the rendered output, to be able to detect changes
h := sha256.New()
_, err = h.Write([]byte(manifest))
if err != nil {
return err
}
new.Checksum = hex.EncodeToString(h.Sum(nil))

// compute resource names
_, resources, err := convertYAMLManifestToJSON(manifest)
if err != nil {
return err
}
new.ResourceNames = resources

return nil
}

func (r *helmReleaseProvider) helmTemplate(ctx context.Context, urn resource.URN, newRelease *Release) (string, error) {
func (r *helmReleaseProvider) helmLoad(ctx context.Context, urn resource.URN, newRelease *Release) (*helmchart.Chart, error) {
conf, err := r.getActionConfig(newRelease.Namespace)
if err != nil {
return "", err
return nil, err
}
client := action.NewInstall(conf)
c, path, err := getChart(&client.ChartPathOptions, conf.RegistryClient, r.settings, newRelease)
if err != nil {
logger.V(9).Infof("getChart failed: %v", err)
logger.V(9).Infof("Settings: %#v", r.settings)
return "", err
return nil, err
}

logger.V(9).Infof("Checking chart dependencies for chart: %q with path: %q", newRelease.Chart, path)

// check and update the chart's dependencies if needed
updated, err := checkChartDependencies(
c,
Expand All @@ -475,68 +453,16 @@ func (r *helmReleaseProvider) helmTemplate(ctx context.Context, urn resource.URN
conf.RegistryClient,
newRelease.DependencyUpdate)
if err != nil {
return "", err
return nil, err
} else if updated {
// load the chart again if its dependencies have been updated
c, err = loader.Load(path)
if err != nil {
return "", err
}
}

logger.V(9).Infof("Fetching values for release: %q", newRelease.Name)
values, err := getValues(newRelease)
if err != nil {
return "", err
}

logger.V(9).Infof("Values: %+v", values)

err = isChartInstallable(c)
if err != nil {
return "", err
}

client.DryRun = true
client.ClientOnly = true
client.Devel = newRelease.Devel
client.DependencyUpdate = newRelease.DependencyUpdate
client.Namespace = newRelease.Namespace
client.ReleaseName = newRelease.Name
client.GenerateName = false
client.NameTemplate = ""
client.OutputDir = ""
client.SubNotes = newRelease.RenderSubchartNotes
client.DisableOpenAPIValidation = true
client.Replace = true
client.APIVersions = nil
client.IncludeCRDs = !newRelease.SkipCrds

if cmd := newRelease.Postrender; cmd != "" {
pr, err := postrender.NewExec(cmd)

if err != nil {
return "", err
}

client.PostRenderer = pr
}

if r.clientSet != nil {
err = setKubeVersionAndAPIVersions(r.clientSet, client)
if err != nil {
return "", err
return nil, err
}
}

logger.V(9).Infof("template helm chart")
rel, err := client.RunWithContext(r.canceler.context, c, values)
if err != nil {
return "", err
}

manifest := getManifest(rel, !newRelease.DisableWebhooks, true)
return manifest, nil
return c, nil
}

func (r *helmReleaseProvider) helmCreate(ctx context.Context, urn resource.URN, newRelease *Release) error {
Expand Down Expand Up @@ -795,17 +721,14 @@ func (r *helmReleaseProvider) Diff(ctx context.Context, req *pulumirpc.DiffReque

// apply ignoreChanges
for _, ignore := range req.GetIgnoreChanges() {
if ignore == "checksum" {
news["checksum"] = oldInputs["checksum"]
if ignore == "version" {
news["version"] = olds["version"]
}
}

// Calculate the diff between the old and new inputs
diff := oldInputs.Diff(news)
if diff == nil {
logger.V(9).Infof("No diff found for %q", req.GetUrn())
return &pulumirpc.DiffResponse{Changes: pulumirpc.DiffResponse_DIFF_NONE}, nil
}
// remove deprecated inputs from old inputs, to avoid producing a delete op w.r.t. the new state.
delete(oldInputs, "checksum")
delete(oldInputs, "resourceNames")

oldRelease, err := decodeRelease(olds, fmt.Sprintf("%s.olds", label))
if err != nil {
Expand All @@ -819,34 +742,33 @@ func (r *helmReleaseProvider) Diff(ctx context.Context, req *pulumirpc.DiffReque
logger.V(9).Infof("Diff: Old release: %#v", oldRelease)
logger.V(9).Infof("Diff: New release: %#v", newRelease)

// Always set desired state to DEPLOYED
// TODO: This could be done in Check instead?
if newRelease.Status == nil {
newRelease.Status = &ReleaseStatus{}
// Generate a patch to apply the new inputs to the old state, including deletions.
oldInputsJSON, err := json.Marshal(oldInputs.MapRepl(nil, mapReplStripSecrets))
if err != nil {
return nil, pkgerrors.Wrapf(err, "internal error: json.Marshal(oldInputsJson)")
}
newRelease.Status.Status = release.StatusDeployed.String()

oldInputsJSON, err := json.Marshal(oldInputs.Mappable())
logger.V(9).Infof("oldInputsJSON: %s", string(oldInputsJSON))
newInputsJSON, err := json.Marshal(news.MapRepl(nil, mapReplStripSecrets))
if err != nil {
return nil, err
return nil, pkgerrors.Wrapf(err, "internal error: json.Marshal(oldInputsJson)")
}
newInputsJSON, err := json.Marshal(news.Mappable())
logger.V(9).Infof("newInputsJSON: %s", string(newInputsJSON))
oldStateJSON, err := json.Marshal(olds.MapRepl(nil, mapReplStripSecrets))
if err != nil {
return nil, err
return nil, pkgerrors.Wrapf(err, "internal error: json.Marshal(oldStateJson)")
}

logger.V(9).Infof("oldInputsJSON: %+v", string(oldInputsJSON))
logger.V(9).Infof("newInputsJSON: %+v", string(newInputsJSON))
patch, err := jsonpatch.CreateMergePatch(oldInputsJSON, newInputsJSON)
logger.V(9).Infof("oldStateJSON: %s", string(oldStateJSON))
strategicPatchJSON, err := strategicpatch.CreateThreeWayMergePatch(oldInputsJSON, newInputsJSON, oldStateJSON, &noSchema{}, true)
if err != nil {
return nil, err
return nil, pkgerrors.Wrapf(err, "internal error: CreateThreeWayMergePatch")
}
logger.V(9).Infof("strategicPatchJSON: %s", string(strategicPatchJSON))
patchObj := map[string]any{}
if err = json.Unmarshal(patch, &patchObj); err != nil {
if err = json.Unmarshal(strategicPatchJSON, &patchObj); err != nil {
return nil, pkgerrors.Wrapf(
err, "Failed to check for changes in Helm release %s because of an error serializing "+
err, "Failed to check for changes in Helm release %s/%s because of an error serializing "+
"the JSON patch describing resource changes",
newRelease.Name)
newRelease.Namespace, newRelease.Name)
}

// Pack up PB, ship response back.
Expand All @@ -873,9 +795,6 @@ func (r *helmReleaseProvider) Diff(ctx context.Context, req *pulumirpc.DiffReque
"converting JSON patch describing resource changes to a diff",
newRelease.Namespace, newRelease.Name)
}
for _, v := range detailedDiff {
v.InputDiff = true
}

for k, v := range detailedDiff {
switch v.Kind {
Expand All @@ -892,7 +811,7 @@ func (r *helmReleaseProvider) Diff(ctx context.Context, req *pulumirpc.DiffReque
DeleteBeforeReplace: false, // TODO: revisit this.
Diffs: changes,
DetailedDiff: detailedDiff,
HasDetailedDiff: true,
HasDetailedDiff: len(detailedDiff) > 0,
}, nil
}

Expand Down Expand Up @@ -1061,9 +980,7 @@ func (r *helmReleaseProvider) Read(ctx context.Context, req *pulumirpc.ReadReque
return nil, err
}

liveInputsPM := r.serializeImportInputs(existingRelease)

inputs, err := plugin.MarshalProperties(liveInputsPM, plugin.MarshalOptions{
inputs, err := plugin.MarshalProperties(oldInputs, plugin.MarshalOptions{
Label: label + ".inputs", KeepUnknowns: true, SkipNulls: true, KeepSecrets: r.enableSecrets,
})
if err != nil {
Expand All @@ -1080,6 +997,7 @@ func (r *helmReleaseProvider) Read(ctx context.Context, req *pulumirpc.ReadReque

func (r *helmReleaseProvider) serializeImportInputs(release *Release) resource.PropertyMap {
inputs := resource.NewPropertyMap(release)
delete(inputs, "resourceNames")
delete(inputs, "status")
return inputs
}
Expand Down Expand Up @@ -1330,11 +1248,13 @@ func (r *helmReleaseProvider) importRelease(ctx context.Context, urn resource.UR
release.Chart = hr.Chart.Metadata.Name
}

err := r.setComputedInputs(ctx, urn, release)
chart, err := r.helmLoad(ctx, urn, release)
if err != nil {
// Likely because the chart is not readily available (e.g. import of chart where no repo info is stored).
// Eat the error to allow import to succeed, assuming that Check will report the failure later.
contract.IgnoreError(err)
} else {
release.Version = chart.Metadata.Version
}

return nil
Expand Down Expand Up @@ -1758,3 +1678,22 @@ func getTimeoutOrDefault(timeout int) time.Duration {
}
return time.Duration(timeout) * time.Second
}

// noSchema implements a trivial lookup function for patch metadata (i.e. patch strategy and merge key).
// CreateThreeWayMergePatch supports various strategies for merging maps and slices, but we use the default strategy.
type noSchema struct {
}

var _ strategicpatch.LookupPatchMeta = &noSchema{}

func (*noSchema) LookupPatchMetadataForSlice(key string) (strategicpatch.LookupPatchMeta, strategicpatch.PatchMeta, error) {
return &noSchema{}, strategicpatch.PatchMeta{}, nil
}

func (*noSchema) LookupPatchMetadataForStruct(key string) (strategicpatch.LookupPatchMeta, strategicpatch.PatchMeta, error) {
return &noSchema{}, strategicpatch.PatchMeta{}, nil
}

func (*noSchema) Name() string {
return ""
}
Loading

0 comments on commit d90d153

Please sign in to comment.