Commit Graph

221 Commits

Author SHA1 Message Date
Hidde Beydals 49639638b9 Rewrite `HelmRepositoryReconciler` to new standards
This commit rewrites the `HelmRepositoryReconciler` 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.
- Refactoring of some Helm elements to make them easier to use within
  the new reconciler logic.
- 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>
2021-08-12 11:47:56 +02:00
Hidde Beydals a0e4f5036b chore: ensure Git server dir is removed after test
Signed-off-by: Hidde Beydals <hello@hidde.co>
2021-08-11 10:44:02 +02:00
Hidde Beydals 29f207d7c8 Wrap err with context instead of logging twice
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>
2021-08-11 09:35:35 +02:00
Sunny b49a809c93 BucketReconciler: Add reconcileArtifact tests
Add `BucketReconciler.reconcileArtifact` tests based on
`GitRepositoryReconciler.reconcileArtifact` test cases.

Signed-off-by: Sunny <darkowlzz@protonmail.com>
2021-08-10 22:15:06 +02:00
Hidde Beydals 15bc9e71b1 Consolidate condition types into `FetchFailed`
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>
2021-08-10 22:15:06 +02:00
Hidde Beydals 588ccbfe99 Rewrite `BucketReconciler` to new standards
This commit rewrites the `BucketReconciler` 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.
- Refactor of reconciler logic, including more efficient detection of
  changes to bucket objects by making use of the etag data available,
  and downloading of object files in parallel with a limited number of
  workers (4).
- 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>
2021-08-10 22:11:04 +02:00
Hidde Beydals e79b5734ad storage: change Artifact checksum to SHA256
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>
2021-08-10 10:30:06 +02:00
Sunny 60771d5b4e
gitrepo: test reconcileArtifact condtns & symlink
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>
2021-08-09 13:32:51 +05:30
Sunny 43f2811215
gitrepo: Add tests for old conditions update
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>
2021-08-08 04:16:01 +05:30
Sunny 47ab15d1c0
gitrepo: reconcileInclude test assertion fixes
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>
2021-08-06 19:36:28 +05:30
Hidde Beydals f6f18030eb
Merge pull request #414 from darkowlzz/gitrepo-reconciler-artifact-tests
controllers: Add more tests for reconcileArtifact
2021-08-03 14:08:52 +02:00
Hidde Beydals 29442ba9bf Tweak logged messages
- 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>
2021-08-03 14:01:18 +02:00
Hidde Beydals a1ed1fc4b4 source: `GetRequeueAfter` in place of `GetInterval`
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>
2021-08-03 13:36:12 +02:00
Hidde Beydals f1de98faf0 Replace %q in messages with '%s'
Signed-off-by: Hidde Beydals <hello@hidde.co>
2021-08-03 12:20:19 +02:00
Sunny be4e85b422
controllers: Add more tests for reconcileArtifact
Fixes error returned from target path validation check and adds more
test cases for TestGitRepositoryReconciler_reconcileArtifact.

Signed-off-by: Sunny <darkowlzz@protonmail.com>
2021-08-02 05:42:32 +05:30
Hidde Beydals e34f79203d storage: strip env specific data during archive
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>
2021-07-30 19:37:02 +02:00
Hidde Beydals f28f86a8ee Ensure rel path never traverses outside Storage
Signed-off-by: Hidde Beydals <hello@hidde.co>
2021-07-30 19:36:38 +02:00
Hidde Beydals 08ce0c95fc Rewrite `GitRepositoryReconciler` to new standards
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>
2021-07-30 19:36:32 +02:00
Hidde Beydals 912e59da1f Refactor `hasArtifactUpdated` into `artifactSet`
Signed-off-by: Hidde Beydals <hello@hidde.co>
2021-07-30 12:55:57 +02:00
Hidde Beydals 82583f2247 Implement new runtime interfaces, prepare testenv
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>
2021-07-30 12:33:18 +02:00
Hidde Beydals c4d7e46b90 Drop deprecated `io/ioutil`
The package has been deprecated since Go 1.16, see:
https://golang.org/doc/go1.16#ioutil

Signed-off-by: Hidde Beydals <hello@hidde.co>
2021-07-29 09:58:00 +02:00
Hidde Beydals f5cb441a82 Take relative paths in account for Bucket revision
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>
2021-07-26 10:54:39 +02:00
Sunny 9825a60b74
Use ObjectKeyFromObject instead of ObjectKey
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>
2021-07-26 04:35:08 +05:30
Hidde Beydals 1f27410b34 Update Helm to v3.6.1
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>
2021-06-18 12:31:23 +02:00
Stefan Prodan b8128cf58b
Reinstate Git cloning timeout
Signed-off-by: Stefan Prodan <stefan.prodan@gmail.com>
2021-06-02 14:21:29 +03:00
Stefan Prodan add5444f16
Fix GitRepository include for nested paths
Signed-off-by: Stefan Prodan <stefan.prodan@gmail.com>
2021-05-28 01:17:40 +03:00
Philip Laine fcf7048992 Add include property to GitRepositories
Signed-off-by: Philip Laine <philip.laine@gmail.com>
Signed-off-by: Philip Laine <philip.laine@xenit.se>
2021-05-11 09:46:50 +02:00
Hidde Beydals 67ebe24873 Split bucket item key by `/` to satisfy matcher
Signed-off-by: Hidde Beydals <hello@hidde.co>
2021-04-30 19:40:32 +02:00
Hidde Beydals 8c27e0ad5a Configure ignore domain for GitRepository rules
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>
2021-04-23 11:01:15 +02:00
Hidde Beydals 917300dc79 Write chart data on identitical values overwrite
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>
2021-04-21 14:50:03 +02:00
Dylan Arbour 4a834e1d2d Add `ValuesFiles` to HelmChart spec
Signed-off-by: Dylan Arbour <arbourd@users.noreply.github.com>
2021-04-19 09:16:53 -04:00
Hidde Beydals b5004a93bc Make Storage#Archive file filtering configurable
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>
2021-04-13 15:34:20 +02:00
Hidde Beydals cca2c4a362 Check ignore matches before Bucket item downloads
Signed-off-by: Hidde Beydals <hello@hidde.co>
2021-04-13 15:34:20 +02:00
Dylan Arbour c0bd4ab7d0 Test values overrides
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>
2021-04-08 18:57:05 -04:00
Stefan Prodan 9a08c0cc52
Add well-known CI configs to exclusion list
Signed-off-by: Stefan Prodan <stefan.prodan@gmail.com>
2021-03-31 14:39:12 +03:00
Michael Bridgen 681ddd5db0 Test RecurseSubmodules
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>
2021-03-31 12:22:10 +01:00
Stefan Prodan 664a568822
Add support for Git submodules with go-git
Signed-off-by: Stefan Prodan <stefan.prodan@gmail.com>
2021-03-30 13:00:13 +03:00
Stefan Prodan f0016cfad1
Enable self-signed certs for go-git
Signed-off-by: Stefan Prodan <stefan.prodan@gmail.com>
2021-03-29 13:23:32 +03:00
Somtochi Onyekwere 2624ba93a3 Record suspension metric
Signed-off-by: Somtochi Onyekwere <somtochionyekwere@gmail.com>
2021-03-17 14:04:21 +01:00
Raffael Sahli 1e19503359
break before default
Signed-off-by: Raffael Sahli <raffael.sahli@doodle.com>
2021-02-09 17:35:02 +01:00
Raffael Sahli bc3c4e2a36
fixes writing chart twice which results in a 0bytes tgz
Signed-off-by: Raffael Sahli <raffael.sahli@doodle.com>
2021-02-09 10:17:07 +01:00
Hidde Beydals fac1afa2a8 Move `git/common` to `git`
Signed-off-by: Hidde Beydals <hello@hidde.co>
2021-02-08 14:05:05 +01:00
Philip Laine c063484761 Add custom certificate validation
Signed-off-by: Philip Laine <philip.laine@gmail.com>
2021-02-08 12:19:22 +01:00
Thomas Runyon 8428054575 Properly escape outer loop for present dependency
To prevent dependencies that are already present locally to be included.

Signed-off-by: Thomas Runyon <runyontr@gmail.com>
2021-02-02 16:16:40 +01:00
Hidde Beydals fcc5fc8d32 typo: 'seperated' -> 'separated'
Signed-off-by: Hidde Beydals <hello@hidde.co>
2021-02-01 15:14:13 +01:00
Hidde Beydals 2c09df6570 Be more verbose about invalid chart name
Includes a change to _not_ requeue after validation failure, as
there is no chance on recovery.

Signed-off-by: Hidde Beydals <hello@hidde.co>
2021-01-29 15:26:54 +01:00
Hidde Beydals 3cdc897236 Assume local dependency without Helm repository
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>
2021-01-28 12:28:19 +01:00
Léo Martinez c1512d34a0
rely on index checksum for HelmRepository reconcile operation
Fixes #256

Signed-off-by: Léo Martinez <leo84.martinez@gmail.com>
2021-01-25 19:11:57 +01:00
Stefan Prodan 207ed99d72
Use LocalObjectReference from fluxcd/pkg/meta v0.7
Signed-off-by: Stefan Prodan <stefan.prodan@gmail.com>
2021-01-21 14:18:42 +02:00
Aurel Canciu f1b5768200
Fix values file override
`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>
2021-01-21 13:40:12 +02:00