fix(windows-agent): Generate test certificates dynamically #398
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This certificate has an expiry date, which is not very convenient for tests.
This way we prevent test coupling.
EduardGomezEscandell
force-pushed
the
fix-expired-certificates
branch
from
a1bea40 to
f504541
Compare
didrocks
reviewed
Nov 14, 2023
| @@ -108,18 +134,15 @@ func TestConnect(t *testing.T) { | |||
| landscapeAgentUID: tc.uid, | |||
| } | |||
|
|
|||
| out, err := os.ReadFile(filepath.Join(golden.TestFixturePath(t), "landscape.conf")) | |||
| in, err := os.ReadFile(filepath.Join(golden.TestFixturePath(t), "landscape.conf")) | |||
| if errors.Is(err, os.ErrNotExist) { | |||
There was a problem hiding this comment.
can we reverse the logic? Like:
- set default
- load override if exists (so changing to err == nil). I agree that we then silence require.NoError for any other kinds of error and fallback to default config. I’m happy to have counter-arguments, but I still find loading default and then overriding being way more logical (I had to reread twice) :)
There was a problem hiding this comment.
You're right, default then override is the Go way :)
| SerialNumber: big.NewInt(1), | ||
| Subject: pkix.Name{ | ||
| CommonName: "CanonicalGroupLimited", | ||
| Country: []string{"US"}, |
There was a problem hiding this comment.
I actually based it on what we use in the app manifest 😅:
Although I think we could set it to Narnia and it'd still work 😄
There was a problem hiding this comment.
Narnia
Thanks for letting me once again feel old :p
There was a problem hiding this comment.
That's from the 1950's, you're not that old!
didrocks
previously approved these changes
Nov 14, 2023
We were modifying the fixtures too much: - find and replace for certificate - appending the hostagent URL section This meant that the config in the fixture bore little similarity to the one being used in the end. I switched to templates for the sake of clarity. Now the fixtures look much more like the final config.
EduardGomezEscandell
force-pushed
the
fix-expired-certificates
branch
from
31f6489 to
48e3800
Compare
|
Applied suggestion and did some squashing. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The certificates used in Landscape tests had an expiration date, which I did not notice. This expiration date has arrived and the tests are now failing.
This PR fixes this by generating the certificate dynamically in every test run.
UDENG-1719