SourceVerifiedCondition is a normal condition, remove it from negative
polarity conditions. Add SourceVerifiedCondition in patch option
WithOwnedConditions.
Also, Update the signature of reconcileInclude() to remove include
being passed and overwritten in the first line. Include is available
as part of the passed source object.
Signed-off-by: Sunny <darkowlzz@protonmail.com>
This wraps the errors which are returned instead of logging them, as
the returned error is logged at the end of the reconcile run.
Signed-off-by: Hidde Beydals <hello@hidde.co>
This commit consolidates the `DownloadFailed` and `CheckoutFailed`
Condition types into a new more generic `FetchFailed` type to simplify
the API and observations by consumers.
Signed-off-by: Hidde Beydals <hello@hidde.co>
Adds test cases for reconcileArtifact to check if old status
conditions are removed after new artifact is created.
Adds a test case to verify that the latest artifact symlink points to
the created artifact.
Signed-off-by: Sunny <darkowlzz@protonmail.com>
This tests the status conditions update in the gitrepository reconciler.
Given a mix of old status conditions, on a successful reconciliation,
the status condition is set to Ready=True.
Signed-off-by: Sunny <darkowlzz@protonmail.com>
Use the created artifact server test storage in reconcileInclude
test's GitRepositoryReconciler and cleanup the created storage.
Fix the test assertions to check the copied artifact directories in
the correct path. Also, update the tests to expect artifacts in the
include `toPath` to exist.
Signed-off-by: Sunny <darkowlzz@protonmail.com>
- Mention the current revision in the up-to-date log message.
- Ensure any error that is "swallowed" (not returned) is logged to
ensure they are visible within the logs, and not just by inspecting
the object.
Signed-off-by: Hidde Beydals <hello@hidde.co>
The problem with `GetInterval()` was that the returned type was of
`metav1.Duration`, while almost anywhere it was used, a type of
`time.Duration` was requested. The result of this was that we had to
call `GetInterval().Duration` all the time, which would become a bit
cumbersome after awhile.
To prevent this, we introduce a new `GetRequeueAfter() time.Duration`
method, which both results the right type, and bears a name that is
easier to remember where the value is used most; while setting the
`Result.RequeueAfter` during reconcile operations.
The introduced of this method deprecates `GetInterval()`, which should
be removed in a future MINOR release.
Signed-off-by: Hidde Beydals <hello@hidde.co>
Fixes error returned from target path validation check and adds more
test cases for TestGitRepositoryReconciler_reconcileArtifact.
Signed-off-by: Sunny <darkowlzz@protonmail.com>
NOTE: This should be amended with the previous commit which has
commented out tests.
Update reconcileSource() to work with the test case where no secret is
set. A minimal auth options is created and used for git checkout.
Update TestGitRepositoryReconciler_verifyCommitSignature() to use the
new git.Commit type.
Update TestGitRepositoryReconciler_reconcileSource_checkoutStrategy to
add skipForImplementation for branch commit test case.
Signed-off-by: Sunny <darkowlzz@protonmail.com>
This commit rewrites the `GitRepositoryReconciler` to new standards,
while implementing the newly introduced Condition types, and trying
to adhere better to Kubernetes API conventions.
More specifically it introduces:
- Implementation of more explicit Condition types to highlight
abnormalities.
- Extensive usage of the `conditions` subpackage from `runtime`.
- Better and more conflict-resilient (status)patching of reconciled
objects using the `patch` subpackage from runtime.
- Proper implementation of kstatus' `Reconciling` and `Stalled`
conditions.
- First (integration) tests that solely rely on `testenv` and do not
use Ginkgo.
There are a couple of TODOs marked in-code, these are suggestions for
the future and should be non-blocking.
In addition to the TODOs, more complex and/or edge-case test scenarios
may be added as well.
Signed-off-by: Hidde Beydals <hello@hidde.co>
This commit ensures all API objects implement the interfaces used by
the runtime package to work with conditions, etc., and prepares the
test suite to work with the `pkg/runtime/testenv` wrapper.
Changes are made in a backwards compatible way (that being: the
existing code can still be build and works as expected), but without
proper dependency boundaries. The result of this is that the API
package temporary depends on the runtime package, which is resolved
when all reconcilers have been refactored and the API package does
no longer contain condition modifying functions.
Signed-off-by: Hidde Beydals <hello@hidde.co>
go-git: Include the commit message in the returned commit object.
libgit2: Set the URL in the checkout error.
Add new method Commit.ShortMessage() for returning short commit
message.
Signed-off-by: Sunny <darkowlzz@protonmail.com>
This avoids skipping the directories when generating the archive
tarball.
This makes it easier to scan directory trees.
Signed-off-by: Kevin McDermott <kevin@weave.works>
Ensure that directories are included.
This avoids skipping the directories when generating the archive
tarball.
This makes it easier to scan directory trees.
Signed-off-by: Kevin McDermott <kevin@weave.works>
Refactor logic to install helper tools into one function in the
Makefile. Add support for envtest to help install tools like kubectl,
etcd which helps users run tests more conveniently.
Signed-off-by: Sanskar Jaiswal <sanskar.jaiswal@weave.works>
As the full version can be used as a label value, the full SHA from the
reference takes up too much space from the 63 characters available in
total.
To mitigate against this, we now take a "short" version of the first 12
characters, which was still unique for the Linux kernel in 2019 with
875.000 commits:
http://git-scm.com/book/en/v2/Git-Tools-Revision-Selection#Short-SHA-1
This should be sufficient to safely detect all changes within the
context of operations.
Signed-off-by: Hidde Beydals <hello@hidde.co>
Test case "Setting valid valuesFile attribute" and the tests around it
aren't isolated and most of the time pass because of the results from
the previous tests being re-read as the test expectation match the
previous test results. Failures are very rare to reproduce, even in
the CI they aren't seen but it failed very frequently on my computer,
especially this specific case because unlike the other cases, there is
just one file to be merged, which invalidates the chart result from
the previous cases.
In order to ensure the test wait for the chart to be updated by its
action and not by any other previous updates, status condition message
seems to be the most reliable way, as it also contains the paths of the
files that were merged.
With this change, I could no longer reproduce the failure on my
computer.
Reordering the tests makes this issue more clear.
Signed-off-by: Sunny <darkowlzz@protonmail.com>
By providing the Generation of the object that is getting reconciled
as version metadata to the builder if any custom values files are
defined, the Artifact revision changes if the specification does,
ensuring consumers of the Artifact are able to react to changes in
values (and perform a release).
Signed-off-by: Hidde Beydals <hello@hidde.co>
This commit introduces a typed `BuildError` to be returned by
`Builder.Build` in case of a failure.
The `Reason` field in combination with `BuildErrorReason` can be used
to signal (or determine) the reason of a returned error within the
context of the build process.
At present this is used to determine the correct Condition Reason, but
in a future iteration this can be used to determine the negative
polarity condition that should be set to indicate a precise failure to
the user.
Signed-off-by: Hidde Beydals <hello@hidde.co>
Dealing with some loose ends around making observations, and code
style.
The loaded byes of a chart are used as a revision to ensure e.g.
periodic builds with unstable ordering of items do not trigger a false
positive.
Signed-off-by: Hidde Beydals <hello@hidde.co>
With all the logic that used to reside in the `controllers` package
factored into this package, it became cluttered. This commit tries to
bring a bit more structure in place.
Signed-off-by: Hidde Beydals <hello@hidde.co>
This commit starts wiring the factored out Helm chart build logic into
the reconciler to ensure, validating the API capabilities.
Signed-off-by: Hidde Beydals <hello@hidde.co>
- Ensure the proper path is garbage collected for libgit2 repositories,
as the `Path` method on the repository object returns the `.git`
directory, and not the root path.
- Ensure the Helm test server does not get swapped during tests,
with as side-effect that no obsolete temporary directories remain.
Signed-off-by: Hidde Beydals <hello@hidde.co>
This changes the format of the Artifact checksum from SHA1 to SHA256 to
mitigate chosen-prefix and length extension attacks, and ensures it can
be used to secure content against malicious modifications.
Source consumers (including our own {kustomize,helm}-controllers)
should ensure the SHA256 of a downloaded artifact matches the
advertised checksum before making use of it.
Signed-off-by: Hidde Beydals <hello@hidde.co>
For signed commit verification, this code errors out on line 303:
✗ GitRepository reconciliation failed: ''PGP public keys secret error: expected pointer, but got nil
Pointer was not initialized with a concrete instance of the Secret struct
Signed-off-by: Kingdon Barrett <yebyen@gmail.com>
This commit changes the `gogit` behavior for commit checkouts,
now allowing one to reference to just a commit while omitting any
branch reference. Doing this creates an Artifact with a
`HEAD/<commit>` revision.
If both a `branch` and `commit` are defined, the commit is expected
to exist within the branch. This results in a more efficient clone
of just the target branch, and also makes this change backwards
compatible.
Fixes#407Fixes#315
Signed-off-by: Hidde Beydals <hello@hidde.co>
This commit refactors the previous `Commit` interface into a
standardised `Commit` struct. This object contains sufficient
information for referencing, observating and (PGP) verification.
- `libgit2` commit checkout does now return `HEAD/<SHA1>` as
the branch is not taken into account.
- `git2go` objects are now properly `Free`d everywhere
- `Verify` logic is tested.
Signed-off-by: Hidde Beydals <hello@hidde.co>
This commit moves the previous `AuthStrategy` wiring to a more generic
`AuthOptions`, breaking free from implementation specific details in
the `git` package.
Signed-off-by: Hidde Beydals <hello@hidde.co>
This commit adds a `ReconcileStrategy` field to the `HelmChart` resource, which
allows defining when a new chart should be packaged and/or published if it
originates from a `Bucket` or `GitRepository` resource.
The two available strategies are:
- `ChartVersion`: creates a new artifact when the version of the Helm chart as
defined in the `Chart.yaml` from the Source is different from the current
version.
- `Revision`: creates a new artifact when the revision of the Source is
different from the current revision.
For the `Revision` strategy, the (checksum part of the) revision of the
artifact the chart originatesfrom is added as SemVer metadata.
A chart from a `GitRepository` with Artifact revision
`main/f0faacd5164a875ebdbd9e3fab778f49c5aadbbc` and a chart with e.g. SemVer
`0.1.0` will be published as `0.1.0+f0faacd5164a875ebdbd9e3fab778f49c5aadbbc`.
A chart from a `Bucket` with Artifact revision
`f0faacd5164a875ebdbd9e3fab778f49c5aadbbc` and a chart with e.g. SemVer `0.1.0`
will be published as `0.1.0+f0faacd5164a875ebdbd9e3fab778f49c5aadbbc`.
Signed-off-by: Dylan Arbour <arbourd@users.noreply.github.com>
Added Support for Google Cloud Storage with Workload Identity as Source Provider. This enables the use of GCP without enabling S3 compatible access.
Signed-off-by: pa250194 <pa250194@ncr.com>
This ensures the checksum is predictable, and not influenced by e.g.
different runtime configuration settings, or FS specific data.
Signed-off-by: Hidde Beydals <hello@hidde.co>
This commit changes the checksum method which is used to calculate the
revision of a Bucket source, so that the file paths are taken into
account and directory structure changes can be observed.
Signed-off-by: Hidde Beydals <hello@hidde.co>
controller-runtime's client package provides ObjectKeyFromObject() to
extract NamespacedName from a given object. ObjectKey() in
internal/util package is a helper for the same. Replace the internal
helper with controller-runtime's helper for the same.
Signed-off-by: Sunny <darkowlzz@protonmail.com>
v3.6.1 is a a security update from Helm, ensuring that credentials are
always only passed to the defined repository host.
Based on Helm user reports, disabling this behavior may be required for
some Helm repository solutions like Artifactory, and may be done by
setting `PassCredentials` in the `HelmRepositorySpec`.
For more information, see:
https://github.com/helm/helm/security/advisories/GHSA-56hp-xqp3-w2jf
Signed-off-by: Hidde Beydals <hello@hidde.co>
Unlike Bucket resources which are matched by key as presented by S3,
ignore rules for GitRepository objects do have a domain: the temporary
directory of the Git repository.
Signed-off-by: Hidde Beydals <hello@hidde.co>
This likely happened because the byte buffer response was already
being read by the chart loader, making it empty by the time the
artifact was written to storage.
As an alternative, and because it makes the code a tiny bit less
obnoxious: write the data to a temp file first, and later decide
what file to copy over and use as an stored artifact.
Signed-off-by: Hidde Beydals <hello@hidde.co>
This commit makes the filtering applied during the archiving
configurable by introducing an optional `ArchiveFileFilter`
callback argument and a `SourceIgnoreFilter` implementation.
`SourceIgnoreFilter` filters out files matching
sourceignore.VCSPatterns and any of the provided patterns.
If an empty gitignore.Pattern slice is given, the matcher is set to
sourceignore.NewDefaultMatcher.
The `GitRepository` now loads the ignore patterns before archiving
the repository contents by calling `sourceignore.LoadIgnorePatterns`
and other helpers. The loading behavior is **breaking** as
`.sourceignore` files in the (subdirectories of the) repository are
now still taken into account if `spec.ignore` for a resource is
defined, overwriting is still possible by creating an overwriting
rule in the `spec.ignore` of the resource.
This change also makes it possible for the `BucketReconciler` to not
configure a callback at all and prevent looking for ignore
matches twice. To finalize the bucket refactor, a change to the
reconciler has been made to look for a `.sourceignore` file in
the root of the bucket to provide an additional way of configuring
(global) exclusions.
Signed-off-by: Hidde Beydals <hello@hidde.co>
Adds a test that loads the helmChart from the updated resource and
verifies that `testOverride` (the value overrode in the test fixtures)
changes from `false` to `true`.
Signed-off-by: Dylan Arbour <arbourd@users.noreply.github.com>
This commit adds a test specifically for RecurseSubmodules. It takes a
bit more preparation, since it needs a repo using submodules to start
with. go-git doesn't appear to support adding submodules
programmatically, so the preparation is done in part by execing `git`.
Signed-off-by: Michael Bridgen <michael@weave.works>
This commit fixes a bug where local chart dependencies would not be
detected correctly due to the absence of a repository URL.
Signed-off-by: Hidde Beydals <hello@hidde.co>
`io.Read` was used incorrectly to read from the override file provided
by the user.
This is now replaced with `ioutil.ReadFile` for better handling and
error reporting.
Fixes#263
Signed-off-by: Aurel Canciu <aurelcanciu@gmail.com>
To include a bug fix to the `ReconcilateAtChangedPredicate`
and renaming to `ReconcileRequestedPredicate`.
Signed-off-by: Hidde Beydals <hello@hidde.co>
This commit upgrades the `controller-runtime` dependency to `v0.7.0`,
including all changes required to make all wiring work again.
- Upgrade `runtime` to v0.6.0 to include `controller-runtime` changes.
- Loggers have been removed from the reconcilers and are now retrieved
from the `context.Context` passed to the `Reconcile` method and
downwards functions.
- Logger configuration flags are now bound to the flag set using
`BindFlags` from `runtime/logger`, ensuring the same contract across
GitOps Toolkit controllers, and the `--log-json` flag has been
deprecated in favour of the `--log-encoding=json` default.
- The `ChangePredicate` from `runtime` has changed to a
`ReconcilateAtChangedPredicate`, and is now chained with the
`GenerationChangedPredicate` from `controller-runtime` using
`predicate.Or`.
- Signatures that made use of `runtime.Object` have changed to
`client.Object`, removing the requirement to e.g. call
`runtime.Object#Object`.
- The `client.MatchingField` function was deprecated, and has been
replaced with `client.MatchingFields{}`.
- The `leader-election-role` was changed, as leader election now works
via the `coordination/v1` API.
Other notable changes:
- `util.ObjectKey` was added to easily construct a `client.ObjectKey` /
`types.NamespacedName` from a `metav1.Object`.
- The `SourceIndexKey` constant has been split out into
`{GitRepository,HelmRepository,Bucket}IndexKey` constants.
Signed-off-by: Hidde Beydals <hello@hidde.co>
As part of the feature implementation to support helm chart
dependencies, the functionality for allowing values files overwriting
from any location scoped to the same source was altered. This should fix
the problem by allowing users to load files from any arbitrary location
as long as it's in the context of the same source from where the helm
chart itself is loaded.
Signed-off-by: Aurel Canciu <aurelcanciu@gmail.com>
The controller logic is a serious candidate to be refactored so that
errors can be taken into account better, and do not always result in a
requeue. But this will do for the time being.
Signed-off-by: Hidde Beydals <hello@hidde.co>
As GCP's S3 interoperability does not implement the ListObjectsV2
implementation.
Ref: https://cloud.google.com/storage/docs/interoperability
> Note: While most actions are interoperable with the Amazon S3 V2 SDK,
> listing objects can only be performed using the Amazon S3 V1 list
> objects method.
Signed-off-by: Hidde Beydals <hello@hidde.co>
Following the rules described in
https://helm.sh/docs/chart_best_practices/conventions/#chart-names.
This guards against people following the wrong guidance of Artifactory,
that supports and promotes repository indexes with e.g. '/' in the
chart names.
In a future version this should be moved to a validation webhook, but
there are still discussions ongoing around the TLS certificates for
this.
Signed-off-by: Hidde Beydals <hello@hidde.co>
Sunny
Hidde Beydals
Paulo Gomes
Kevin McDermott
Stefan Prodan
Sanskar Jaiswal
Tom Huang
Aurel Canciu
Kingdon Barrett
pa250194
Dylan Arbour
Philip Laine
Michael Bridgen
Somtochi Onyekwere
Raffael Sahli
Philip Laine
Thomas Runyon
Léo Martinez