Previously, the BSL credential field would always be set when using the
`create` command, even if no credential details were provided. This
would result in an empty `SecretKeySelector` in the BSL which would
cause operations using this BSL to fail as Velero would attempt to fetch
a `Secret` with an empty name from the K8s API server.
With this change, the `Credential` field is only set if credential
details have been specified. This change also includes some refactoring
to allow the change to be tested.
Signed-off-by: Bridget McErlean <bmcerlean@vmware.com>
Velero was including DownwardAPI volumes when backing up with restic.
When restoring these volumes, it triggered a known issue with restic (as
seen in #3863). Like projected volumes, these volumes should be skipped
as their contents are populated by the Kubernetes API server.
With this change, we are now skipping the restic backup of volumes with
a DownwardAPI source. We are also skipping the restore of any volume
that had a DownwardAPI source as there will exist backups that were
taken prior to this fix being introduced. This will allow these backups
to be restored succesfully.
Signed-off-by: Bridget McErlean <bmcerlean@vmware.com>
Instead of converting the unstructured item to check for the presence of
the `kube-aggregator.kubernetes.io/automanaged` label, use this label in
the `AppliesTo` to enable the restore logic to select the item. This
means that any item that matches the selector will have restore skipped.
Also add a new test case to the restore action test to check that label
selectors are applied correctly.
Signed-off-by: Bridget McErlean <bmcerlean@vmware.com>
It was discovered during Velero 1.6.3 upgrade testing that Velero was
restoring `APIService` objects for APIs that are no longer being served
by Kubernetes 1.22. If these items were restored, it would break the
behaviour of discovery within the cluster.
This change introduces a new RestoreItemAction plugin that skips the
restore of any `APIService` object which is managed by Kubernetes such
as those for built-in APIs or CRDs. The `APIService`s for these will be
created when the Kubernetes API server starts or when new CRDs are
registered. These objects are identified by looking for the
`kube-aggregator.kubernetes.io/automanaged` label.
Signed-off-by: Bridget McErlean <bmcerlean@vmware.com>
* Use appropriate CRD API during readiness check
The readiness check for the Velero CRDs was still using the v1beta1 API.
This would cause the readiness check to fail on 1.22 clusters as the
v1beta1 API is no longer available. Previously, this error would be
ignored and the installation would proceed, however with #4002, we are
no longer ignoring errors from this check.
This change modifies the CRD readiness check to check the CRDs using the
same API version that was used when submitting the CRDs to the cluster.
It also introduces a new CRD builder using the V1 API for testing.
This change also fixes a bug that was identified in the polling code
where if the CRDs were not ready on the first polling iteration, they
would be added again to the list of CRDs to check resulting in
duplicates. This would cause the length check to fail on all subsequent
polls and the timeout would always be reached.
Signed-off-by: Bridget McErlean <bmcerlean@vmware.com>
* Remove duplicate V1 CRD builder and update comment
Signed-off-by: Bridget McErlean <bmcerlean@vmware.com>
The `push-build-image` target was broken in #3634. The `ifneq`
conditional block had tabs for indentation which results in incorrect
behaviour. Instead, remove whitespace before the conditional block like
we do for other similar blocks.
Signed-off-by: Bridget McErlean <bmcerlean@vmware.com>
This adds a new `buildinfo` variable `ImageRegistry` that can set at
build time like the `Version` variable. This allows us to customise the
Velero binary to use different registries.
If the variable is set, this variable wille be used when creating the
URIs for both the main `velero` and `velero-restic-restore-helper`
images. If it is not set, default to using Dockerhub (`velero/velero`,
`velero/velero-restic-restore-helper`).
There are numerous ways in which the Velero binary can be built so all
of them have been updated to add the new link time flag to set the
variable:
* `make local` (used for local developer builds to build for the local
OS and ARCH)
* `make build` (used by developers and also VMware internal builds to
build a specific OS and ARCH)
* Goreleaser config (used when creating OSS release binaries)
* Dockerfile (used to build the Velero binary used within the image)
All of these workflows are currently triggered from our Makefile where
the variable `REGISTRY` is already available with the default value of
`velero` and used to build the image tag. Where the new `ImageRegistry`
build variable is needed, we pass through this Makefile variable to
those tasks so it can be used accordingly.
The GitHub action and the `./hack/docker-push.sh` script used to push
container images has not been modified. This will continue to use the
default registry specified in the Makefile and will not explicitly pass
it in.
Signed-off-by: Bridget McErlean <bmcerlean@vmware.com>
Wenkai Yin recently joined the Velero team within VMware. He has been
contributing to the technical health of Velero, introducing important
changes such as running our E2E tests as part of our PR checks and will
continue to focus in this area.
Signed-off-by: Bridget McErlean <bmcerlean@vmware.com>
Daniel Jiang recently joined the Velero team within VMware and will be
taking on a technical leadership role. He has been contributing to the
project through community engagement including issue triage and
community support, and is taking on more significant feature development
within Velero such as the design and development of the `velero debug`
feature.
Signed-off-by: Bridget McErlean <bmcerlean@vmware.com>
In #3863, it was discovered that volumes from projected sources were
being backed up by restic when they should have been skipped. Restoring
these volumes triggers a known bug in restic.
In #3866, we started skipping volumes from a projected source, however
there will exist backups that were taken before this fix was introduced.
This change modifies the restore logic to skip the restore of any volume
that came from a projected source, allowing backups taken before #3866
to be restored successfully.
Signed-off-by: Bridget McErlean <bmcerlean@vmware.com>
* Add document describing manual test cases
This introduces a new document, `TESTING.md`, which describes manual
tests that are currently run as part of a Velero release and test cases
that we will want to introduce for future releases.
Signed-off-by: Bridget McErlean <bmcerlean@vmware.com>
* Move testing requirements doc to website
Signed-off-by: Bridget McErlean <bmcerlean@vmware.com>
For internal builds of Velero, we need to be able to specify an
alternative Dockerfile which uses an alternative image registry to pull
the base images from. This change adapts our Makefile such that both the
main Dockerfile and build image Dockerfile can be overridden.
We have some special handling for the build image to only build when the
Dockerfile has changed. In this case, we check whether a custom
Dockerfile has been provided, and always rebuild in that case. For
custom build image Dockerfiles, use a fixed tag rather than the one
based on commit SHA of the original file.
Signed-off-by: Bridget McErlean <bmcerlean@vmware.com>
The test for multiple credentials assumed that the plugin for the
additional BSL provider was already installed. This will not be the case
when performing a clean install of Velero between tests.
This adds a new utility function to add the plugins that are necessary
for the additional BSL provider. It doesn't check which plugins are
already installed, it will just attempt to install and if the stderr
contains the message that it is a duplicate plugin, we ignore the error
and continue. This could be improved by instpecting the output from
`velero plugin get` but I opted for a quicker solution given the
upcoming release.
Signed-off-by: Bridget McErlean <bmcerlean@vmware.com>
Our previous render hook to create links would drop the fragment when
linking to headings within the current page or within other markdown
pages on the site.
This change parses the URL and formats the link correctly if it includes
a fragment. If the link is a header on the current page, it is rendered
as `http://<current-url>/#header`. If the link is a header on a
different page (e.g. page.md#header), it is rendered as
`http://<page-url>/#header`.
This change is taken from the following Hugo community support post:
https://discourse.gohugo.io/t/markdown-render-hooks-github-and-hugo-compatible-links/22543/14
Signed-off-by: Bridget McErlean <bmcerlean@vmware.com>
* Use Credential from BSL for restic commands
This change introduces support for restic to make use of per-BSL
credentials. It makes use of the `credentials.FileStore` introduced in
PR #3442 to write the BSL credentials to disk. To support per-BSL
credentials for restic, the environment for the restic commands needs to
be modified for each provider to ensure that the credentials are
provided via the correct provider specific environment variables.
This change introduces a new function `restic.CmdEnv` to check the BSL
provider and create the correct mapping of environment variables for
each provider.
Previously, AWS and GCP could rely on the environment variables in the
Velero deployments to obtain the credentials file, but now these
environment variables need to be set with the path to the serialized
credentials file if a credential is set on the BSL.
For Azure, the credentials file in the environment was loaded and parsed
to set the environment variables for restic. Now, we check if the BSL
has a credential, and if it does, load and parse that file instead.
This change also introduces a few other small improvements. Now that we
are fetching the BSL to check for the `Credential` field, we can use the
BSL directly to get the `CACert` which means that we can remove the
`GetCACert` function. Also, now that we have a way to serialize secrets
to disk, we can use the `credentials.FileStore` to get a temp file for
the restic repo password and remove the `restic.TempCredentialsFile`
function.
Signed-off-by: Bridget McErlean <bmcerlean@vmware.com>
* Add documentation for per-BSL credentials
Signed-off-by: Bridget McErlean <bmcerlean@vmware.com>
* Address review feedback
Signed-off-by: Bridget McErlean <bmcerlean@vmware.com>
* Address review comments
Signed-off-by: Bridget McErlean <bmcerlean@vmware.com>
We are no longer adding the Credentials field to the VSL so this reverts
part the change that added it (#3409).
The original PR also added the `snapshot-location set` command. This
command only included options for setting the credential but is part of
the work for #2426. Due to this, the command has been left in place
(with the credentials option removed) but has been hidden.
Signed-off-by: Bridget McErlean <bmcerlean@vmware.com>
This change adds an E2E test which exercises the mulitple credentials
feature added in #3489. The test creates a secret from the given
credentials and creates a BackupStorageLocation which uses those
credentials. A backup and restore is then performed to the default
BSL and to the newly created BSL.
This change adds new flags to the E2E test suite to configure the BSL
created and used in the test.
Signed-off-by: Bridget McErlean <bmcerlean@vmware.com>
* Load credentials and pass to ObjectStorage plugins
Update NewObjectBackupStore to take a CredentialsGetter which can be
used to get the credentials for a BackupStorageLocation if it has been
configured with a Credential. If the BSL has a credential, use that
SecretKeySelector to fetch the secret, write the contents to a temp file
and then pass that file through to the plugin via the config map using
the key `credentialsFile`. This relies on the plugin being able to use
this new config field.
This does not yet handle VolumeSnapshotLocations or ResticRepositories.
Signed-off-by: Bridget McErlean <bmcerlean@vmware.com>
* Address code reviews
Add godocs and comments.
Improve formatting and test names.
Signed-off-by: Bridget McErlean <bmcerlean@vmware.com>
* Address code reviews
Signed-off-by: Bridget McErlean <bmcerlean@vmware.com>
HTTPS requests were failing due to the ca-certificates package not being
installed in the Tilt image.
This change takes the command to install this package from our main
Dockerfile (which also includes installing tzdata).
Signed-off-by: Bridget McErlean <bmcerlean@vmware.com>
* Use pod namespace from backup when matching PVBs
In #3051, we introduced an additional check to ensure that a PVB matched
a particular pod by checking both the name and the namespace of the pod.
This caused an issue when using a namespace mapping on restore. In the
case where a namespace mapping is being used, the check for whether a
PVB matches a particular pod will fail as the PVB was created for the
original pod namespace and is not aware of the new namespace mapping
being used. This resulted in PVRs not being created for pods that were
being restored into new namespaces. The restic init containers were
being created to wait on the volume restore, however this would cause
the restored pods to block indefinitely as they would be waiting for a
volume restore that was not scheduled.
To fix this, we use the original namespace of the pod from the backup to
match the PVB to the pod being restored, not the new namespace where
the pod is being restored into.
Fixes#3467.
Signed-off-by: Bridget McErlean <bmcerlean@vmware.com>
* Explain why the namespace mapping can't be used
Signed-off-by: Bridget McErlean <bmcerlean@vmware.com>
* Use kubebuilder client for fetching restic secrets
Instead of using a SecretInformer for fetching secrets for restic, use
the cached client provided by the controller-runtime manager.
In order to use this client, the scheme for Secrets must be added to the
scheme used by the manager so this is added when creating the manager in
both the velero and restic servers.
This change also refactors some of the tests to add a shared utility for
creating a fake controller-runtime client which is now used among all
tests which use that client. This has been added to ensure that all
tests use the same client with the same scheme.
Signed-off-by: Bridget McErlean <bmcerlean@vmware.com>
* Add builder for SecretKeySelector
Signed-off-by: Bridget McErlean <bmcerlean@vmware.com>
The labeler action was failing as it was looking for
`.github/labels.yaml` but the file has the suffix `.yml`. This change
fixes the path used by the labeler action.
Signed-off-by: Bridget McErlean <bmcerlean@vmware.com>
With #3327, the restic binary for the Tilt Velero image is downloaded on
the local machine using the `./hack/download-restic.sh` script. This
script relies on `wget` being availabe on the local machine. `wget` is
not commonly available on macOS but `curl` is. This change modifies the
`./hack/download-restic.sh` script to use `curl` instead as it is
available on both Linux and macOS and is available in our `golang`
docker build image.
Signed-off-by: Bridget McErlean <bmcerlean@vmware.com>
In preparation for modifying the instantiation of `BackupStores` to be
able to load credentials, change the function `NewObjectBackupStore` to
be an interface that is passed in to all controllers.
Previously, the function to get a new backup store was configurable but
for many controllers was fixed to use `NewObjectBackupStore`. This
change introduces an interface for getting the backup store and wraps
the functionality from `NewObjectBackupStore` in a type which implements
this interface. This will allow more flexibility when introducing
credentials for a specific backup store as it will allow us to create a
new `ObjectBackupStoreGetter` type which can be configured to add
credentials config when creating the ObjectBackupStore without needing
to change the API used by the controllers.
Signed-off-by: Bridget McErlean <bmcerlean@vmware.com>
In #3310, the Dockerfile for the Tilt Velero container was modified to
call the `./hack/download-restic.sh` script. A side effect of this
change was that the context for the docker build was much larger as it
was the root of the Velero repo, rather than just the `_tiltbuild`
directory. With the frequent rebuilds of the image that happen when
using Tilt, a large amounts of disk space was being consumed by the
different layers of images builds in the Docker overlay filesystem (as
diffs could include the `.go` directory which can be several GBs).
This change modifies the `download-restic.sh` script to allow the output
directory for the restic binary to be configured. This means that the
script can be called directly from the Tiltfile and can be managed
outside the container build. This allows us to restore the previous
`_tiltbuild` context. It also speeds up image builds as we can download
restic once and use it for all builds rather than redownloading
frequently.
Signed-off-by: Bridget McErlean <bmcerlean@vmware.com>
This change customises the issue template chooser to include a link to
the Community Support Q&A discussion board. This lets users know that
there is another place to ask questions related to using Velero.
This change also disables the creation of blank issues to prevent issues
that don't follow either the bug or feature request templates from being
opened.
Signed-off-by: Bridget McErlean <bmcerlean@vmware.com>
* Add Tilt configuration to debug using Delve
This change adds support to run the Velero process in Tilt using
[Delve](https://github.com/go-delve/delve).
This does not include support for debugging the Velero process in the
restic pods, just in the Velero deployment.
For an optimal debugging experience, this change also introduces a new
flag (`DEBUG`) to the `hack/build.sh` script to enable a "debug" build
of the Velero binary. This flag, if enabled, will build the binary
without optimisations and inlining. Disabling optimisations and inlining
is recommended by Delve.
Two configuration options have been added to the Tilt settings. The
first, `enable_debug`, is to control whether debugging should be
enabled. If enabled, the process will be started by Delve, and the Delve
server port (2345) will be forwarded to the local machine.
The second option, `debug_continue_on_start`, is to control whether the
process should "continue" when started by Delve or should be paused.
By default, debugging is disabled, and if in debug mode, the process
will continue.
Signed-off-by: Bridget McErlean <bmcerlean@vmware.com>
* Add spaces around keyword args
Starlark prefers spaces around `=` in keyword arguments:
https://docs.bazel.build/versions/master/skylark/bzl-style.html#keyword-arguments
Signed-off-by: Bridget McErlean <bmcerlean@vmware.com>
* Remove unnecessary command from Dockerfile
Signed-off-by: Bridget McErlean <bmcerlean@vmware.com>
* Add note to connect after Tilt is running
Signed-off-by: Bridget McErlean <bmcerlean@vmware.com>
This change adds an additional set of commands to Dockerfile for the
Velero image which adds the `hack/download-restic.sh` script, installs
the necessary dependencies, and then runs that script.
In order to copy the script from the `hack` directory, the context for
building the image has been changed to the root of the velero
repository.
Signed-off-by: Bridget McErlean <bmcerlean@vmware.com>
This changes the codespell action config to use a relative path for the
generated crds.go file as the current pattern used fails the check used
by codespell (which uses the `fnmatch` module).
Signed-off-by: Bridget McErlean <bmcerlean@vmware.com>
PR #3110 introduced a new action for performing the login to Dockerhub
as part of image building and pushing however there is an error with the
configuration and the credentials are not being passed through
correctly. This change reverts to the previous log in approach.
Signed-off-by: Bridget McErlean <bmcerlean@vmware.com>
The previous buildx action that we were using has been archived and
users are recommended to switch to the new action provided by Docker.
The previous action also included setting up QEMU. This is now provided
as a separate action which needs to be run separately.
This change also replaces the direct use of `docker login` with the new
`login-action`. This new action also handles logging out once the build
is complete.
Signed-off-by: Bridget McErlean <bmcerlean@vmware.com>
* Don't fail backup if downloading tarball fails
Previously, we would always attempt to download the tarball for a backup
for processing DeleteItemAction plugins, even if there weren't any.
This caused an issue for some users in the case where the backup tarball
had been deleted from object storage as the backup deletion would fail.
Now, we only attempt to download the tarball in the case where there are
DeleteItemAction plugins. If downloading that tarball fails, we log
the error, skip the processing of the DeleteItemAction plugins and
proceed with the rest of the deletion.
Signed-off-by: Bridget McErlean <bmcerlean@vmware.com>
* Skip file removal in closeAndRemoveFile if nil
Signed-off-by: Bridget McErlean <bmcerlean@vmware.com>
This change modifies the kubebuilder annotations for the Velero CRDs to
include `additionalPrinterColumns` so that more information is exposed
when using `kubectl get`.
For each of the CRDs, annotations have been added to make the output
for `kubectl get` match the output from the equivalent `velero get`
command as closely as possible. There are some cases where this output
could not be replicated, such as the `EXPIRES` column for Backups, due
to the limitations of JSONPath expressions within the resulting CRD
defition. Some columns undergo processing and formatting before being
printed by the Velero CLI which cannot be replicated using JSONPath. In
these cases, these printer columns have been omitted.
For other CRDs where there is no `velero get` equivalent, such as
`PodVolumeBackup` and `PodVolumeRestore`, a best effort has been made to
expose information that provides value.
Signed-off-by: Bridget McErlean <bmcerlean@vmware.com>
The command to check for an existing release branch only checked for
local branches. We should be considering both local and remote branches
before cherry-picking commits for the new release.
This change checks for existing local and remote release branches and
creates or updates them accordingly.
* If a remote branch exists, but a local branch does not, checkout the
remote branch and track it.
* If the remote branch and local branch exists, checkout the local
branch and ensure that the latest commits from the remote are pulled.
* Otherwise, if the remote branch does not exist, create it locally if
needed.
This also handles the case where an existing release branch may be
tracked in multiple remotes as the remote to use is explicitly stated.
Signed-off-by: Bridget McErlean <bmcerlean@vmware.com>
We instruct users to update the CRDs when upgrading to 1.4 and 1.5 which
involves using `kubectl apply` to apply the CRD configuration. The CRD
configuration generated by `velero install` includes fields which are
not valid when running Kubernetes v1.14 or earlier. We instruct users to
work around this when doing a customised velero install, but not when
upgrading to newer versions. This change updates the upgrade
instructions for v1.4 and v1.5 to include the use of `--validate=false`
flag when running `kubectl apply`.
See #2077 and #2311 for more context.
Signed-off-by: Bridget McErlean <bmcerlean@vmware.com>
The release script assumes that the remote for the vmware-tanzu/velero
repository is called `upstream`. It may be the case that this remote is
configured to use a different name. This change updates the script to
allow the remote name being used to be configured by setting the
environment variable `REMOTE` before running the script. If the variable
is not set, the remote defaults to `upstream`.
The release instructions have also been updated to reflect this change.
Signed-off-by: Bridget McErlean <bmcerlean@vmware.com>
This change addresses some issues in the documentation and scripts that
were found during the v1.5.1 release:
* Fix the path to the changelog script in the Makefile
* Fix the path to the pre-release TOC in the docs
* Improve the instructions for creating/updating the upgrade
instructions page.
Signed-off-by: Bridget McErlean <bmcerlean@vmware.com>
Now that Exec restore hooks have been added in #2804 and are available
in 1.5.0-rc1, we can remove the line that states that they are coming
soon.
Signed-off-by: Bridget McErlean <bmcerlean@vmware.com>
A number of links still pointed to the old master branch and resulted in
404s. This updates those links to point to the new main branch.
Signed-off-by: Bridget McErlean <bmcerlean@vmware.com>
Bridget McErlean
