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.

Sign up for GitHub

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

Jump to bottom

[infra-tests] Add self-signed integration test #12910

Merged
merged 1 commit into from
Sep 20, 2022
Merged

[infra-tests] Add self-signed integration test #12910

merged 1 commit into from
Sep 20, 2022

Conversation

Pothulapati
Copy link
Contributor

@Pothulapati Pothulapati commented Sep 13, 2022 •
edited
Loading

Description

This PR adds a new test option called self-signed which is used to
enable self-signed integration tests. This is done by adding a new
generate-self-signed make option, which is called whenever self_signed
env variable is enabled.

Once enabled, self-signed certs are genereated and relevant self-signed-config
is called to attach those files into kots config.

Signed-off-by: Tarun Pothulapati [email protected]

Related Issue(s)

Fixes #11270

How to test

Run

werft run github -f -s .werft/installer-tests.ts -j .werft/eks-installer-tests.yaml -a debug=true -a self-signed=true -a skipTests=true -a preview=true

Release Notes

[infra-tests] Add self-signed integration test

Documentation

Werft options:

  • /werft with-preview

nandajavarma
nandajavarma reviewed Sep 13, 2022
View reviewed changes
.werft/installer-tests.ts Outdated
@@ -18,9 +18,10 @@ const version: string = annotations.version || "-";
const preview: string = annotations.preview || "false"; // setting to true will not destroy the setup
const upgrade: string = annotations.upgrade || "false"; // setting to true will not KOTS upgrade to the latest version. Set the channel to beta or stable in this case.
const skipTests: string = annotations.skipTests || "false"; // setting to true skips the integration tests
const selfSigned: string = annotations.selfSigned || "false";
Copy link
Contributor

Choose a reason for hiding this comment

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

SelfSigned annotation passed via job makes sense, but when running cron, this doesn't get toggled automatically, so I recommend doing a randomize on [true, false] for this boolean. So it rotates for tests.

Suggested change
const selfSigned: string = annotations.selfSigned || "false";
const selfSigned: string = annotations.selfSigned || randomize(["true", "false"]);

@Pothulapati Pothulapati force-pushed the tar/self-signed-test branch 10 times, most recently from 2ca9054 to 4bc6a89 Compare September 14, 2022 10:51
This was referenced Sep 15, 2022
Closed
Closed
@Pothulapati Pothulapati marked this pull request as ready for review September 19, 2022 04:57
@Pothulapati Pothulapati requested a review from a team September 19, 2022 04:57
@github-actions github-actions bot added the team: delivery Issue belongs to the self-hosted team label Sep 19, 2022
@Pothulapati
Copy link
Contributor Author

With #11811, I've communicated that self-signed isn't supported in any cloud provider with the new single cluster reference guides. We should be able to still merge this for testing purposes right? @nandajavarma

@Pothulapati Pothulapati changed the title [infra-tests] Add a self-signed infra phase [infra-tests] Add self-signed integration test Sep 19, 2022
nandajavarma
nandajavarma requested changes Sep 19, 2022
View reviewed changes
.werft/jobs/build/job-config.ts Outdated Show resolved Hide resolved
install/tests/Makefile Outdated
@@ -111,10 +111,14 @@ add-ns-record: check-env-cloud
.PHONY:
## cluster-issuer: Creates a cluster issuer for the correspondign provider
cluster-issuer: check-env-cloud
ifndef self_signed
Copy link
Contributor

Choose a reason for hiding this comment

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

I might be missing something here, but if self_signed is not defined, skip cluster issuer? 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

you can do something like https://github.com/gitpod-io/gitpod/pull/12910/files#diff-bd7b7f3a397df93111a6b6e2a4284b01bd57e04474bc9f91f1b02ee1960399a2L163

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated to use ifneq

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But wondering if this means, self-signed is now triggered even when for -a self-signed=false (which I'm not sure anyone would do though). ifneq with a boolean seems to err for some reason. 🤔

Copy link
Contributor

@nandajavarma nandajavarma Sep 20, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I feel like I might have been a bit short-sighted on my suggestion. I thought self-signed is either set or not since you were using ifndef. So, basically self-signed is a string that can have default value "false" or user provided value(can be "true", can be something else). So would it work if we did ifneq($(self_signed),"false") ?
I promise I will unblock you soon 🙈
Edit: I do not see any boolean values used

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So, using ifneq for boolean strings doesn't feel great. Instead, I'm now setting self_signed only when needed, and hence using ifdef. Trying that out now. Sorry for the back and forth!

install/tests/Makefile Outdated Show resolved Hide resolved
@Pothulapati Pothulapati force-pushed the tar/self-signed-test branch 2 times, most recently from 3e32b37 to 3daf524 Compare September 19, 2022 13:09
@Pothulapati Pothulapati force-pushed the tar/self-signed-test branch 6 times, most recently from 207426e to 761d399 Compare September 20, 2022 05:17
@Pothulapati Pothulapati force-pushed the tar/self-signed-test branch 4 times, most recently from c568241 to 1cae09f Compare September 20, 2022 07:52
@Pothulapati
[infra-tests] Add self-signed integration test
f9a2ff8 
This PR adds a new test option called `self-signed` which is used to
enable `self-signed` integration tests. This is done by adding a new
`generate-self-signed` make option, which is called whenever `self_signed`
env variable is enabled.

Once enabled, self-signed certs are genereated and relevant `self-signed-config`
is called to attach those files into `kots` config.

Signed-off-by: Tarun Pothulapati <[email protected]>
@roboquat roboquat merged commit fe26d26 into main Sep 20, 2022
@roboquat roboquat deleted the tar/self-signed-test branch September 20, 2022 08:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nandajavarma nandajavarma nandajavarma approved these changes

Assignees
No one assigned
Labels
release-note size/L team: delivery Issue belongs to the self-hosted team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Automated testing with Self-signed certs
3 participants
@Pothulapati @nandajavarma @roboquat