Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[installer]: create tests to verify render result #11288

Merged
merged 4 commits into from
Aug 2, 2022
Merged

Conversation

mrsimonemms
Copy link
Contributor

@mrsimonemms mrsimonemms commented Jul 11, 2022

Description

This is a proposed change to allow for testing of the rendered YAML in the Installer.

The data is stored in cmd/testdata/render, with the test names being stored in cmd/testdata/render/<name>/<config|output>.golden. Files ending .golden are excluded from the hooks to make them testable. The intention is that there will be a suite of these tests and they verify that the known output is matched. When a change is made (even outside of this directory), the files will require changing and @gitpod-io/engineering-self-hosted is notified about potentially breaking changes to the rendered YAML - we can then approve (or not) the changes.

How to generate the files is documented.

Randomisation is controlled by specifying the Seed for math/rand and by giving the crypto/rand a known string. A warning is printed to stderr if this is enabled during the YAML rendering process, which is designed to act as a deterrent to anyone using this in production.

Risks Introduced

It's not impossible for a PR to pass its tests in a branch and then fail when it's merged. This can be mitigated by getting into the habit of doing a final git rebase main before this is merged.

In the event of a failure in the main branch, the engineer who authored the original PR is responsible for ensuring that this is fixed in a timely fashion.

Related Issue(s)

Fixes #11147

How to test

Run tests in Werft. See also the docs

Release Notes

[installer]: create tests to verify render result

Documentation

Werft options:

  • /werft with-preview

@mrsimonemms mrsimonemms force-pushed the sje/render-testing branch 2 times, most recently from 28a8e24 to 922feff Compare July 11, 2022 16:43
@mrsimonemms mrsimonemms changed the title Sje/render testing [installer]: create tests to verify render result Jul 12, 2022
@gitguardian
Copy link

gitguardian bot commented Jul 12, 2022

⚠️ GitGuardian has uncovered 5 secrets following the scan of your pull request.

Please consider investigating the findings and remediating the incidents. Failure to do so may lead to compromising the associated services or software components.

🔎 Detected hardcoded secrets in your pull request
GitGuardian id Secret Commit Filename
4097242 Generic High Entropy Secret 962222a install/installer/cmd/testdata/render/minimal/output.golden View secret
4097242 Generic High Entropy Secret 962222a install/installer/cmd/testdata/render/minimal/output.golden View secret
4097243 Generic High Entropy Secret 962222a install/installer/cmd/testdata/render/minimal/output.golden View secret
4097242 Generic High Entropy Secret 962222a install/installer/cmd/testdata/render/minimal/output.golden View secret
4097244 Generic High Entropy Secret 962222a install/installer/cmd/testdata/render/minimal/output.golden View secret
🛠 Guidelines to remediate hardcoded secrets
  1. Understand the implications of revoking this secret by investigating where it is used in your code.
  2. Replace and store your secret safely. Learn here the best practices.
  3. Revoke and rotate this secret.
  4. If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.

To avoid such incidents in the future consider


🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

Our GitHub checks need improvements? Share your feedbacks!

@stale
Copy link

stale bot commented Jul 30, 2022

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the meta: stale This issue/PR is stale and will be closed soon label Jul 30, 2022
@lucasvaltl lucasvaltl removed the meta: stale This issue/PR is stale and will be closed soon label Aug 1, 2022
Simon Emms added 3 commits August 2, 2022 10:23
This allows for the values to be consistent when testing
This is useful for testing so we can generate the same rendering
each time and then compare the outputs against a known value. It also
allows users to make their randomisation deterministic
@mrsimonemms mrsimonemms force-pushed the sje/render-testing branch 2 times, most recently from f22f217 to 15daf85 Compare August 2, 2022 10:37
@mrsimonemms mrsimonemms marked this pull request as ready for review August 2, 2022 11:45
@mrsimonemms mrsimonemms requested a review from a team August 2, 2022 11:45
@github-actions github-actions bot added the team: delivery Issue belongs to the self-hosted team label Aug 2, 2022
Copy link
Contributor

@corneliusludmann corneliusludmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! Thanks for implementing this! 🚀

@roboquat roboquat merged commit e9a1376 into main Aug 2, 2022
@roboquat roboquat deleted the sje/render-testing branch August 2, 2022 12:02
@iQQBot
Copy link
Contributor

iQQBot commented Aug 3, 2022

@mrsimonemms Looks like this PR is block our add new components, because installer render is not stable, i.e. some random value, some random order

Also, I try this command, and it failed by a weird error, but I can manually download this file

image

image

@mrsimonemms
Copy link
Contributor Author

Hi @iQQBot, to get through this you'll need to run make deps inside /install/installer as the tests don't download the Helm dependencies at runtime.

I'll create a PR to auto-download these as part of the workspace creation

@iQQBot
Copy link
Contributor

iQQBot commented Aug 3, 2022

OK, thanks, but I'm a little curious, the order of render looks random, how does it ensure that the order is consistent each time it is rendered?

@mrsimonemms
Copy link
Contributor Author

It is random how it's generated, but we sort the generated YAML - see https://github.com/gitpod-io/gitpod/blob/main/install/installer/cmd/render.go#L219

We also have an option to set the random values to a deterministic state - in the tests, we just set that to 42. See https://github.com/gitpod-io/gitpod/blob/main/install/installer/cmd/root.go#L42,L53

@mrsimonemms
Copy link
Contributor Author

Fix created #11829

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note size/XXL team: delivery Issue belongs to the self-hosted team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[installer] Allow setting the rand seed as CLI argument for testing purposes
5 participants