Skip to content
This repository has been archived by the owner on Aug 28, 2024. It is now read-only.

Feature: Neuvector UI test #270

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Feature: Neuvector UI test #270

wants to merge 5 commits into from

Conversation

TheFutonEng
Copy link

This PR is the result of the work from the Platform Testing of Zarf Package team. The goal of this work was to test out the theory that the best place to put the testing of a Zarf package is embedded with the Zarf package.

A simple podinfo example was done as part of a previous Dash Days, and then the thought was, "Hey, let's try DUBBD next, right?" - @blancharda

Forgetting the infinite complexity that exists between those two deployments, the team soldiered on, and we targeted a simple UI test for Neuvector in the K3d-specific DUBBD package. The changes in this PR provide a simple template that can be followed for this type of testing.

local_port=62202


kubectl port-forward -n $namespace service/$svc_name $local_port:$svc_port > /dev/null 2>&1 &
Copy link
Collaborator

Choose a reason for hiding this comment

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

could you please update this to use ./zarf tools kubectl since we don't currently require kubectl to be installed? thanks!

Copy link
Author

Choose a reason for hiding this comment

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

@MxNxPx , will do.

Copy link
Author

Choose a reason for hiding this comment

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

@MxNxPx , change made, script still works:

$ ./test-http-response.sh neuvector neuvector-service-webui 8443
neuvector-service-webui is up and reachable.

Copy link
Collaborator

@mjnagel mjnagel left a comment

Choose a reason for hiding this comment

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

Thinking through how this fits into DUBBD more generally - we likely wouldn't want testing to be part of the published content that users consume? I think this would be more of something for our own CI probably, in which case maybe its in its own zarf package?

k3d/zarf.yaml Outdated Show resolved Hide resolved
local_port=62202


zarf tools kubectl port-forward -n $namespace service/$svc_name $local_port:$svc_port > /dev/null 2>&1 &
Copy link
Collaborator

Choose a reason for hiding this comment

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

In the context of DUBBD I wonder if its better to hit the istio endpoints (i.e. neuvector.bigbang.dev), but that pulls in its own world of fun 🙃

Copy link
Author

Choose a reason for hiding this comment

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

I think we briefly considered that and now can't remember why we decided to go direct to the service...

@blancharda, do you recall?

Co-authored-by: Micah Nagel <[email protected]>
Signed-off-by: Rob Mengert <[email protected]>
@TheFutonEng
Copy link
Author

Thinking through how this fits into DUBBD more generally - we likely wouldn't want testing to be part of the published content that users consume? I think this would be more of something for our own CI probably, in which case maybe its in its own zarf package?

@mjnagel, we were trying to prove that it is possible through actions to embed testing of the application in a Zarf package within the package itself. We think this PR proves that. If you feel this type of test for DUBBD specifically is better suited for CI, that works for me. We think that the pattern is useful for package development in general.

FYI - @blancharda @bm54cloud

@mjnagel
Copy link
Collaborator

mjnagel commented Jun 16, 2023

@mjnagel, we were trying to prove that it is possible through actions to embed testing of the application in a Zarf package within the package itself. We think this PR proves that. If you feel this type of test for DUBBD specifically is better suited for CI, that works for me. We think that the pattern is useful for package development in general.

@TheFutonEng - that makes sense and I love the work done here. It might just be a decision on whether we want this type of testing to run on deploy by an actual user, or just on deploy in CI/dev. And maybe I'm thinking this through too much since the addition here is just to the k3d package, which we assume to be consumed for non-prod 😅.

@UncleGedd any thoughts on whether or not we might want to bring these types of things into DUBBD (or more specifically DUBBD-k3d)?

@UncleGedd
Copy link
Contributor

Yes, I love the concept and I think we should bring this sort of thing into CI for all of our packages. IMO it should be done with an actual testing framework and not with shell scripts, BUT, understood that we don't have a testing framework today. Also, agreed with @mjnagel that this should probably be in CI not when the user deploys the package.

@TheFutonEng
Copy link
Author

@UncleGedd, @mjnagel, I believe the group looked at TerraTest a little during a previous dash days but not sure what the outcome was (@blancharda?). Not sure if that's a fit for DUBBD.

Given this dialogue in this thread, please feel free to close/reject the PR (whatever you think is best).

FYI: @bm54cloud

@UncleGedd
Copy link
Contributor

I'm totally cool with keeping it, the only requested change is to run it in the CI. How hard would it be to add the rest of the services in addition to Neuvector?

@blancharda
Copy link
Contributor

I actually think there's a huge amount of value in having the tests built into the package itself so that any deployment (CI or otherwise) can be validated inherently as part of a zarf package deploy. This is especially true if it's the same tests that are used in CI to test the package.

Perhaps making the component optional is a reasonable middle ground?

And yes, @mjnagel it would be better if we could test against the istio endpoints.. but it felt like there are too many external (and unsafe) assumptions involved with doing so. Namely that the gateway service is guaranteed to be assigned an address (ex.. it's not guaranteed in k3d without metal-lb) and we would have to assume the address itself is routable. That might be workable in CI, but for package validation in the wild I don't think we can make that assumption.

@blancharda
Copy link
Contributor

@UncleGedd it should be pretty straightforward to add the other services, but we believe that should happen in the base package. We only tested Neuvector here because it was the only "external" service modified by the k3d values/local-values.yaml. The intent here is to make sure that the changes to the bigbang values didn't impact our ability to access the exposed services.

If we want to move forward with this kind of test workflow, it might make sense to add checks for all the exposed apps in the base package.

@UncleGedd
Copy link
Contributor

any deployment (CI or otherwise) can be validated inherently as part of a zarf package deploy

👍 good point, I agree!

it was the only "external" service modified by the k3d

Understood, thanks for explaining the reasoning. Agree that if the configs aren't being modified by a particular distro (ie. k3d, AWS), they should be tested in the base package

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants