When #8156 was merged it had the side effect that all hooks were
run all the time. All the hooks were put in the flow of the
content rendered and sent to Kubernetes on every command.
For example, if you ran the following 2 commands the test hooks
would run:
helm create foo
helm install foo ./foo
This should not run any hooks. But, the generated test hook is run.
The change in this commit moves the writing of the hooks to output
or disk back into the template command rather than in a private
function within the actions. This is where it was for v3.2.
One side effect is that post renderers will not work on hooks. This
was the case in v3.2. Since this bug is blocking the release of v3.3.0
it is being rolled back. A refactor effort is underway for this section
of code. post renderer for hooks should be added back as part of that
work. Since post renderer hooks did not make it into a release it
is ok to roll it back for now.
There is code in the cmd/helm package that has been duplicated from
pkg/action. This is a temporary measure to fix the immediate bug
with plans to correct the situation as part of a refactor
of renderResources.
Signed-off-by: Matt Farina <matt@mattfarina.com>
Two things changed in this commit...
1. The Build behavior was restored and the change only impacts
Update. This is a more minimal functionality change thats
a more secure behavior
2. Cleanup from Josh's feedback on the PR to create a const
and comment changes
Signed-off-by: Matt Farina <matt@mattfarina.com>
If a repository was not know to helm (e.g. added using helm repo add)
then Helm would use the range set in the depenencies as the version
in the lock file. Lock files should not have ranges since they are
locked to versions.
Helm did this because the version information for repositories was
not know to Helm. This change fixes that by making the repository
and chart information known to Helm so it can resolve the versions.
Closes#8449
Signed-off-by: Matt Farina <matt@mattfarina.com>
* fix(sdk): Polish the downloader/manager package error return
Close#8471
Signed-off-by: Dong Gang <dong.gang@daocloud.io>
* Modify the repositories validation function `resloveRepoNames` and add a
unit test.
Signed-off-by: Dong Gang <dong.gang@daocloud.io>
* Remove wrong commit
Signed-off-by: Dong Gang <dong.gang@daocloud.io>
If two `helm upgrade`s are executed at the exact same time, then one of
the invocations will fail with "already exists".
If one `helm upgrade` is executed and a second one is started while the
first is in `pending-upgrade`, then the second invocation will create a
new release. Effectively, two helm invocations will simultaneously
change the state of Kubernetes resources -- which is scary -- then two
releases will be in `deployed` state -- which can cause other issues.
This commit fixes the corrupted storage problem, by introducting a poor
person's lock. If the last release is in a pending state, then helm will
abort. If the last release is in a pending state, due to a previously
killed helm, then the user is expected to do `helm rollback`.
Closes#7274
Signed-off-by: Cristian Klein <cristian.klein@elastisys.com>
* fix(template):Issue:helm template with --output-dir doesn't write template with a hook to file
Close#7836
Signed-off-by: Dong Gang <dong.gang@daocloud.io>
* fix go file style
Signed-off-by: Dong Gang <dong.gang@daocloud.io>
* fix go file style
Signed-off-by: Dong Gang <dong.gang@daocloud.io>
Previously, the `helm ls --$state` operation would display outdated
releases under certain conditions.
Given the following set of releases:
```
NAME REVISION UPDATED STATUS CHART APP VERSION NAMESPACE
bar 1 Wed Apr 8 16:54:39 2020 DEPLOYED bar-4.0.0 1.0 default
foo 1 Fri Feb 7 06:16:56 2020 DEPLOYED foo-0.1.0 1.0 default
foo 2 Mon May 4 07:16:56 2020 FAILED foo-0.1.0 1.0 default
foo 3 Mon May 4 07:20:00 2020 FAILED foo-0.1.0 1.0 default
foo 4 Tue May 5 08:16:56 2020 DEPLOYED foo-0.2.0 1.0 default
qux 1 Tue Jun 9 10:32:00 2020 DEPLOYED qux-4.0.3 1.0 default
qux 2 Tue Jun 9 10:57:00 2020 FAILED qux-4.0.3 1.0 default
```
`helm ls --failed` produced the following output:
```
NAME REVISION UPDATED STATUS CHART APP VERSION NAMESPACE
foo 3 Mon May 4 07:20:00 2020 FAILED foo-0.1.0 1.0 default
qux 2 Tue Jun 9 10:57:00 2020 FAILED qux-4.0.0 1.0 default
```
Including the `qux` release in that `helm ls --failed` output is not
controversial; the most recent revision of `qux` was not successful
and an operator should investigate.
Including the `foo` release in the output, however, is
questionable. Revision 3 of `foo` is _not_ the most recent release of
`foo`, and that FAILED release was fixed in a susubsequent upgrade. A
user may see that FAILED deploy and start taking inappropriate
action. Further, that issue was fixed months ago in this example --
troubleshooting an old deploy may not be safe if significant changes
have occurred. Concern over this behavior was raised in
https://github.com/helm/helm/issues/7495.
This behavior applied to all the state filter flags (--deployed,
--failed, --pending, etc.), and a user could pass multiple state
filter flags to a single command. The previous behavior can be
summarized as follows:
For each release name, all release revisions having any of the
supplied state flags were retrieved, and the most recent revision
among these was returned (regardless of whether a newer revision of an
unspecified state exists).
This change request alters the helm list action to match user
expectations such that only "current" releases are shown when
filtering on release state. After this change, the following output
would be produced by `helm ls --failed`:
```
NAME REVISION UPDATED STATUS CHART APP VERSION NAMESPACE
qux 2 Tue Jun 9 10:57:00 2020 FAILED qux-4.0.0 1.0 default
```
The command now returns only `qux` because it is the only "current" FAILED release.
This behavior change applies to all the state filters _except_
`superseded`, which now becomes a special case. By definition, at
least one newer release exists ahead of each superseded release. A
conditional is included in this change request to maintain the
preexisting behavior (return "most recent" superseded revison for
each release name) if the superseded state filter is requested.
---
Note that there is an alternate perspective that a state filter flag
should return all releases of a given state rather than only the
"current" releases. In the above example, `helm ls --failed` with this
approach would return the following:
```
NAME REVISION UPDATED STATUS CHART APP VERSION NAMESPACE
foo 2 Mon May 4 07:16:56 2020 FAILED foo-0.1.0 1.0 default
foo 3 Mon May 4 07:20:00 2020 FAILED foo-0.1.0 1.0 default
qux 2 Tue Jun 9 10:57:00 2020 FAILED qux-4.0.0 1.0 default
```
Multiple FAILED `foo` revisions are included in the output, unlike the current behavior.
This approach is logical and achievable. It allows a user to find
exactly what is requested: all historical releases of a given
state. In order to achieve continuity with helm behavior, however, a
new filter (something like "current") would probably need to be
implemented and become the new default.
Given current helm behavior as well as the comments in the #7495, I
did not pursue this approach.
---
Technical details:
- Moved list action state mask filter after latest release filter
Previously, the list operation in helm/pkg/action/list.go skipped
releases that were not covered by the state mask on _retrieval_ from
the Releases store:
```
results, err := l.cfg.Releases.List(func(rel *release.Release) bool {
// Skip anything that the mask doesn't cover
currentStatus := l.StateMask.FromName(rel.Info.Status.String())
if l.StateMask¤tStatus == 0 {
return false
}
...
```
8ea6b970ec/pkg/action/list.go (L154-L159)
While filtering on retrieval in this manner avoided an extra iteration
through the entire list to check on the supplied condition later, it
introduced the possibility of returning an outdated release to the
user because newer releases (that would have otherwise squashed
outdated releases in the `filterList` function) are simply not
included in the set of working records.
This change moves the state mask filtering process to _after_ the set
of current releases is built. Outdated, potentially misleading
releases are scrubbed out prior to the application of the state mask
filter.
As written, this state mask filtration (in the new `filterStateMask`
method on `*List`) incurs an additional, potentially expensive
iteration over the set of releases to return to the user. An
alternative approach could avoid that extra iteration and fit this
logic into the existing `filterList` function at the cost of making
`filterList` function a little harder to understand.
- Rename filterList to filterLatestReleases for clarity
Another function that filters the list is added, so update
to the more descriptive name here.
- List superseded releases without filtering for latest
This change makes superseded releases a special case, as they would
_never_ be displayed otherwise (by definition, as superseded releases have been
replaced by a newer release), so a conditional maintains current
behavior ("return newest superseded revision for each release name")
Fixes#7495.
Signed-off-by: Andrew Melis <andrewmelis@gmail.com>
If stat returns an error other than the directory not existing
it was unhandled. When IsDir is called in one of these situations
it causes a panic.
Closes#8181
Signed-off-by: Matt Farina <matt@mattfarina.com>
Since Tiller is no longer part of Helm v3, internal documentation
language about Tiller can be removed
Signed-off-by: Matt Farina <matt@mattfarina.com>
* Fixing issue with PAX headers in plugin archive
PAX Headers can be added by some systems that create archives. Helm
should ignore them when extracting.
There are two PAX headers. One is global and the other is not. Both
are ignored. The test adds only the PAX global header because the
Go tar package is unable to write the header that is not global.
Closes#8084
Signed-off-by: Matt Farina <matt@mattfarina.com>
* Removing the PAX header test as it is not working
The PAX header test was making a WriteHeader call and ignoring the
error. When writing the type TypeXHeader it was causing an error
that was being silently ignored. The Go tar package cannot write
this type and produces an error when one tries to. The error reads
"cannot manually encode TypeXHeader, TypeGNULongName, or TypeGNULongLink
headers"
Signed-off-by: Matt Farina <matt@mattfarina.com>
* Adding check of returned error in test
Adding a check for the returned error to make sure a non-nil value
is not returned.
Signed-off-by: Matt Farina <matt@mattfarina.com>
* fix: make the linter coalesce the passed-in values before running values tests
Signed-off-by: Matt Butcher <matt.butcher@microsoft.com>
* fixed typo
Signed-off-by: Matt Butcher <matt.butcher@microsoft.com>
Add api group:
- apiextensions.k8s.io/v1beta1
- rbac.authorization.k8s.io/v1alpha1
Also, some kinds moved from extensions/v1 to extensions/v1beta1
Signed-off-by: Martin Hickey <martin.hickey@ie.ibm.com>
A chart being installed which only contains CRDs and not
any templates tries to install the resources by default.
The resourceList which is used in this case does not check
if there are resources present in it or not. This commit
adds checks to those particular places where we need to check
if the size of resourceList > 0 during installation and deletion.
Signed-off-by: Vibhav Bobade <vibhav.bobde@gmail.com>
Helm had been exposing XDG based variables to end users. This lead
to confusion. For example, if a user wanted to change the cache
location Helm used should they change the XDG variable? Since this
would be like changing the HOME environment variable the answer
is no.
This change adds HELM_*_HOME environment variables to be used
in addition to XDG ones of the same name. Helm will now look
for the Helm specific variable. If not set, Helm will fall
back to XDG locations. If those are not set a default location
will be used. This keeps XDG in use as a default when present,
provides users with the ability to set the location, and removes
XDG from being exposed to end users to avoid confusion.
Closes#7919
Signed-off-by: Matt Farina <matt@mattfarina.com>
* fix: write index.yaml file atomically
This refactors the already-existing `AtomicWriteFile` utility
to a central location and uses it to write index files
atomically.
This is done to avoid having half-written index files break
client requests.
Drive-bys:
- Add test for AtomicWriteFile.
- Add test IndexFile.WriteFile.
Signed-off-by: rabadin <rvbadin@gmail.com>
* Review fix: use RenameWithFallback instead of os.Rename
Signed-off-by: rabadin <rvbadin@gmail.com>
Co-authored-by: rabadin <rvbadin@gmail.com>
In the created chart from `helm create` is notes a tag overrides
version. It actually overrides appVersion. Updating the docs
to reflect reality.
Signed-off-by: Matt Farina <matt@mattfarina.com>
Removes the BOM prefix if present, in read files before
processing the data.
Affects the following pkg:
- pkg/chart/loader: directory and archive loader
- internal/ignore: when loading .helmignore file
Signed-off-by: Thomas FREYSS <thomas.freyss@gmail.com>
* return the new values if modifications dont yet exist
Signed-off-by: David Pait <DP19@users.noreply.github.com>
* fix tests
Signed-off-by: David Pait <DP19@users.noreply.github.com>
* removed outter if statement as its not needed now
Signed-off-by: David Pait <DP19@users.noreply.github.com>
* Parse reference templates in predictable order
Fix issue #7701
Signed-off-by: Andre Sencioles <asenci@gmail.com>
* Add test case for issue #7701 regression
Signed-off-by: Andre Sencioles <asenci@gmail.com>
* gofmt
Signed-off-by: Andre Sencioles <asenci@gmail.com>
Note, there is an issue with a dependency of sprig changing
behavior. A test has been added with a description to catch if a
behavior breaking change of mergo is used.
See https://github.com/imdario/mergo/issues/139 for the mergo
issue and sprig for further details on handling this in the
future.
Closes #7533
Signed-off-by: Matt Farina <matt@mattfarina.com>
The error message returned from Kubernetes when APIs are
removed is not very informative. This PR adds additional
information to the user. It covers the current release manifest
APIs.
Partial #7219
Signed-off-by: Martin Hickey <martin.hickey@ie.ibm.com>
* Continue deleting objects when one fails to minimize the risk of an
upgrade ending in an unrecoverable state
* Exclude failed deleted object from the returned result set
Signed-off-by: Adam Reese <adam@reese.io>
* test(pkg/storage/secrets): make MockSecretsInterface.List follow ListOptions
Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com>
* test(pkg/storage/secrets): add unit test for Secrets.Query
Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com>
* test(pkg/storage/cfgmaps): make MockConfigMapsInterface.List follow ListOptions
Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com>
* test(pkg/storage/cfgmaps): add unit test for ConfigMaps.Query
Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com>
* docs: Update inline docs on action/upgrade.go
Signed-off-by: Matt Butcher <matt.butcher@microsoft.com>
* clarify atomic and cleanup-on-fail
Signed-off-by: Matt Butcher <matt.butcher@microsoft.com>
* updated the post-render documentation on action.Upgrade
Signed-off-by: Matt Butcher <matt.butcher@microsoft.com>
Upgrade Kubernetes libraries to v0.18.0
Add new lazy load KubernetesClientSet to avoid missing kubeconfig error
In kubernetes v1.18 kubeconfig validation was added. Minikube and Kind
both remove kubeconfig when stopping clusters. This causes and error
when running any helm commands because we initialize the client before
executing the command.
Signed-off-by: Adam Reese <adam@reese.io>
* fix: fixed bug in Dependency.List()
A bug in Dependency.List() caused all compressed charts to flag their dependencies as "missing".
Closes#4431
Signed-off-by: Matt Butcher <matt.butcher@microsoft.com>
* removed some files from test fixtures
Signed-off-by: Matt Butcher <matt.butcher@microsoft.com>
The version field in the Chart.yaml has a comment describing it
but it did not note the version needs to follow SemVer. There
have been numerous questions, over time, about this format. Add
note here so it's exposed in more places.
Signed-off-by: Matt Farina <matt@mattfarina.com>
While using the chart version as image tag is the sanest default, it is not uncommon to want to override this if using a custom image, or using helm to manage an in-house app running different tags across different environments.
Signed-off-by: Naseem <naseem@transit.app>
* fix: update unit test for go 1.14 error string change
Signed-off-by: Matt Butcher <matt.butcher@microsoft.com>
* changed strategy based on conversation with Adam
Signed-off-by: Matt Butcher <matt.butcher@microsoft.com>
- Removed most right whitespace chomps except those directly following a
template definition where it make sense to not lead with a blank line.
The system applied is now to almost always left whitespace chomp but
also whitespace chomp right if its the first thing in a file or
template definition.
- Updated indentation to be systematic throughout all the boilerplace
files.
Signed-off-by: Erik Sundell <erik.i.sundell@gmail.com>
The 'helm upgrade' command was not checking if the cluster was reachable.
Also, 'helm upgrade --install' first checks if the release exists
already. If that check fails there is no point in continuing the
upgrade. This optimization avoids a second timeout of 30 seconds when
trying to do the upgrade.
Signed-off-by: Marc Khouzam <marc.khouzam@montreal.ca>
When archives are created on windows the path spearator in the
archive file is \\. This causes issues when the file is unpacked.
For example, on Linux the files are unpacked in a flat structure
and \ is part of the file name. This causes comp issues. In Helm
v2 the path was set as / when the archive was written. This works
on both Windows and POSIX systems.
The fix being implemented is to use the ToSlash function to ensure
/ is used as the separator.
Fixes#7748
Signed-off-by: Matt Farina <matt@mattfarina.com>
If several instances of Helm are run at the same moment and try to download the
same chart, some of them might see an empty or incomplete file in cache. Prevent
that by saving the dowloaded file atomically.
Closes#7600
Signed-off-by: Mikhail Gusarov <misha@ridge.co>
* fix: add new static linter and fix issues it found
Signed-off-by: Matt Butcher <matt.butcher@microsoft.com>
* fixed two additional linter errors.
Signed-off-by: Matt Butcher <matt.butcher@microsoft.com>
It is more idiomatic to compare the string against the empty string than to check the string's length.
Signed-off-by: Matthew Fisher <matt.fisher@microsoft.com>
Don't delete a resource on upgrade if it is annotated with
helm.io/resource-policy=keep. This can cause data loss for users
if the annotation is ignored(e.g. for a PVC)
Close#7677
Signed-off-by: Dong Gang <dong.gang@daocloud.io>
This complements the verification output fixed in #7706. On verify
there should be some detail about the verification rather than
no information.
Signed-off-by: Matt Farina <matt@mattfarina.com>
When using the --verify flag on the pull command the output was
an internal Go object rather than useful detail. This is a bug.
The output new displays who signed the chart along with the
hash.
Fixes#7624
Signed-off-by: Matt Farina <matt@mattfarina.com>
The memory driver is used for go tests. It can also be used from the
command-line by setting the environment variable HELM_DRIVER=memory.
In the latter case however, there was no way to pre-provision some
releases.
This commit introduces the HELM_MEMORY_DRIVER_DATA variable which
can be used to provide a colon-separated list of yaml files specifying
releases to provision automatically.
For example:
HELM_DRIVER=memory \
HELM_MEMORY_DRIVER_DATA=./testdata/releases.yaml \
helm list --all-namespaces
Signed-off-by: Marc Khouzam <marc.khouzam@montreal.ca>