SANDBOX-858: make getAndPrint exportable

[rsoaresd]

Description

This PR makes getAndPrint exportable. It will be useful for wa

Issue ticket number and link

SANDBOX-858

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[mfrancisc]

minor - should we move the packages used by WA to toolchain-common or some new library/repo ?

I'm asking this since importing from toolchain-e2e might be problematic - for example in case toolchain-e2e and WA depend on different versions of api and toolchain-common. It is very unlikely that this might happen with the current workflow, since we have the habit to keep those up to date in all repositories, but there is no restriction or enforcement on doing that.

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alexeykazakov, mfrancisc, rsoaresd

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [alexeykazakov,mfrancisc]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[MatousJobanek]

Good point @mfrancisc.
Theoretically, moving the code to toolchain-common would make complete sense and would follow what we've been doing in all other cases till now.
However, practically it wouldn't make much of a difference in this case. WA code will rely also on other parts of the toolchain-e2e code (makefile targets, WaitForDeployments func, awatitilities, and maybe more). And moving all these things to toolchain-common doesn't make much sense.
Also, when you think about the usage from the WA point of view, it's something different compared to the KubeSaw components. WA uses toolchain-e2e to setup KubeSaw in the e2e environment, it doesn't use it to test KubeSaw functionality, so there is no risk of messing up with the proper separation of concerns.

That being said, I think it's fine to have the direct dependency from WA to toolchain-e2e repo

[mfrancisc]

WA code will rely also on other parts of the toolchain-e2e code

Got it , thanks for explaining. I see that there is no value ATM in doing this separation.

One other thing to consider ( a part from the api , toolchain-common dependency ) is that - changing some of these parts in toolchain-e2e that are now shared with WA, might break WA without noticing that. Until someone tries out WA or opens a PR in there.

[MatousJobanek]

there are two types of dependencies:

1. setup in makefile target (which uses always the master version of toolchain-e2e)
2. in go code where the version of the toolchain-e2e (that is being used) is defined in go.mod

The only thing that could break WA e2e test is the first one. If there is some change in the setup that is not compatible with:

• either the way WA calls toolchain-e2e
• or the version of the toolchain-e2e go code WA uses

but there is nothing we could do about it. So, I wouldn't worry about it

[mfrancisc]

in go code where the version of the toolchain-e2e (that is being used) is defined in go.mod

this type of dependency could break as well, if you need to update the toolchain-e2e version and the new version has changes affecting the functions used by WA.

What I'm saying is that there is no way for someone doing changes on toolchain-e2e to figure out if the changes are still compatible with how WA uses this functionality, or to know which functionality is actually used in WA, unless it opens a PR in WA with the new version of toolchain-e2e.
But again - I understand that extracting those reusable functionality to a common library/repo might be premature now, and that getting WA to a production state has higher priority right now. For now we can live with it knowing that there is toolchain-e2e functionality used in WA, and it's worth checking before/while working on changes for the e2e tests ( same thing as we do when working at changes for api and toolchain-common repo ) .

[rsoaresd]

@mfrancisc @MatousJobanek, thanks a lot for this discussion and your insights!

I thought about reusing toolchain-e2e make targets and functions (WaitForDeployments, NewSignupRequest, etc.) to not duplicate "unnecessary" code on wa side and to make the implementation faster. As both (make targets and these functions) are something that we do not change often, the risk of something going wrong is less. But, of course, the risk is there.

The "most critical" aspect is indeed the setup, as @MatousJobanek mentioned. We rely on the deploy-single-member-e2e to deploy the toolchain e2e resources on wa side.

I think we are good for now. But, in the future, we need to reconsider if we should move the logic to a common repo.

[alexeykazakov]

I would try to re-use as much form the e2e repo in the wa tests as possible. So +1 to not duplicating the code but depending on / reusing it.
However it mean that we can always break something. But it's not different than depending on common or anything else (I'm talking about go.mod dependencies, not make targets). You can now commit something into common which would break downstream dependencies IF you upgrade the dependency to the latest common.

[mfrancisc]

I think we are good for now. But, in the future, we need to reconsider if we should move the logic to a common repo.

Thanks for the background @rsoaresd 🙏
I agree it's not a problem for now !

[mfrancisc]

I would try to re-use as much form the e2e repo in the wa tests as possible. So +1 to not duplicating the code but depending on / reusing it.

@alexeykazakov I agree, we should avoid duplication, I was wondering HOW we should reuse it, if keeping it in toolchain-e2e or moving the reusable stuff to some library repo ( like common ). Mainly thinking at the scenario in which someone working at e2e tests and not familiar with WA codebase, changes things that could break WA, without any automation or any way to know that before the code review.

You can now commit something into common which would break downstream decencies IF you upgrade the dependency to the latest common.

Not a deal breaker right now I agree!

I just wanted to point out that it's slightly different IMO because common is not a project but a set of reusable packages. When we change functionality from common we search for the usage in all the downstream dependencies and we keep those compatible. We also introduced make verify-dependencies in both common and api to automate this check.

In addition , if I'm not mistaken, the version of api and common used by WA must be the same one used in toolchain-e2e with this setup, and thus we need to keep them in synch, but I might be wrong as I'm not familiar with WA codebase yet.

But again, I agree this is the quicker way to move forward without duplicating code, it's not something we should worry about now, maybe later if it turns out to be problematic I would say 👍