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

add a default user-agent header #892

Merged
merged 12 commits into from May 16, 2024

Conversation

catdevman
Copy link
Contributor

@catdevman catdevman commented Mar 28, 2024

Checklist
  • make test-all (UNIX) passes. CI will also test this
  • unit and/or integration tests are included (if applicable)
  • documentation is changed or added (if applicable)

Description of change

Close #882. Adding default user-agent header by detecting if a user-agent header was given in the request_headers field in the goss file and defaulting to goss/x.x.x. Any guidance that can be given on how to get the current version of goss within the codebase would be appreciated. I saw app.Version but that seemed to be related to urfave/cli so wasn't sure I could get that from that far down. The easiest way is probably adding it to the context but I didn't want to bloat that since it would go to all checks not just http.

@catdevman
Copy link
Contributor Author

@aelsabbahy any suggestions on how to get the current version of goss down into the NewDefHTTP function?

@aelsabbahy
Copy link
Member

Apologies, somehow I've completely missed this.

I will review over the weekend and get back to you.

@aelsabbahy
Copy link
Member

aelsabbahy commented Apr 18, 2024

To answer your question on version, it's currently here:

var version string

-ldflags "-X main.version=${version_stamp} -s -w" \

Perhaps moving it to a shared version pkg and having both main and this import the shared pkg/var. If there's a better way, feel free to suggest it.

…build.sh script; use util.Version to make user-agent for http action
@catdevman
Copy link
Contributor Author

@aelsabbahy I thought about this a little bit and since the util package is already included in the http code perhaps it might make sense to add it there. I didn't change how main.version was getting set, instead I just added to that process. I tested by making a goss file that called an "echo server" that spat out whatever headers got sent. I am not sure of the best way to test this... maybe integrating with a service like https://echo.free.beeceptor.com/ or spinning up an "echo server" to test against within the pipeline? or maybe this doesn't need tested? Let me know what you're thinking.

Copy link
Member

@aelsabbahy aelsabbahy left a comment

Choose a reason for hiding this comment

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

This looks great. Let's also change main.Version to also use util, so there's one source of truth.

The goss integration tests use httpbin.org, might be able to just add it here:

http://httpbin/headers:

system/http.go Outdated Show resolved Hide resolved
Copy link
Member

@aelsabbahy aelsabbahy left a comment

Choose a reason for hiding this comment

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

Looks good, minor comment. Also, travis-ci seems to be not working, I'll take a look at that.

integration-tests/goss/goss-shared.yaml Outdated Show resolved Hide resolved
Moved header tests into 1
@aelsabbahy
Copy link
Member

CI failed due to linting issues:

invalid gofmt:
cmd/goss/goss.go
make: *** [Makefile:49: fmt] Error 1

https://app.travis-ci.com/github/goss-org/goss/jobs/621547789#L307

@catdevman
Copy link
Contributor Author

Yeah I shouldn't be surprised I tried editing it through GitHub because I wasn't at a computer. 🤦‍♂️

@aelsabbahy
Copy link
Member

Yeah I shouldn't be surprised I tried editing it through GitHub because I wasn't at a computer. 🤦‍♂️

You will receive zero judgement from me, been there 😅

@catdevman
Copy link
Contributor Author

@aelsabbahy I know it was just because of whatever character was in there but it does crack me up a little bit that in that last run Windows was like "looks good to me brother" and mac and linux are saying "hold on a moment something is not quite right" 😆

@aelsabbahy
Copy link
Member

Once the test is in place, this PR is ready to merge.

Noticed the test was removed so waiting on merging it.

@catdevman
Copy link
Contributor Author

Well I was trying to figure out why Linux was failing but perhaps that is expected behavior in the pipeline?

@aelsabbahy
Copy link
Member

No problem, I added the test back. The issue is httpbin returns the headers as pascal case but the goss.yaml was lowercase so it wasn't matching.

Copy link
Member

@aelsabbahy aelsabbahy left a comment

Choose a reason for hiding this comment

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

Thank you for taking the time and effort to add this. Great addition!

@aelsabbahy aelsabbahy merged commit 1b523c0 into goss-org:master May 16, 2024
1 check passed
@catdevman
Copy link
Contributor Author

@aelsabbahy ah I believe that's a different error then I was getting locally or perhaps what I was getting just didn't make sense for it to be this error.
Either way glad to see it merged and happy to have been of service. I've never gotten to use goss professionally but I always thought it was a brilliant bit of tech.

@aelsabbahy
Copy link
Member

Curious, do you use it for personal projects? If so, what's your primary use-case?

Always fun learning how others are using Goss.

Fun note, after initially releasing Goss, I didn't use it personally or professionally for 1-2 years.

@catdevman
Copy link
Contributor Author

I've not found a personal use. I worked in DevOps for a while and I was going to use it for health checks and maybe even regression test for a rest API but they hired a test engineer and he scrapped that for his own thing that did exactly what goss does but it took him 6 months to just get started in it. Then I left right as the whole software division closed up. That was the closest I got. But I have some new ideas and I think eventually these projects could benefit from goss. Mostly as a way of regression testing. Essentially boot up a golden DB and test if the responses have that is expected in them.

dklimpel added a commit to dklimpel/goss that referenced this pull request May 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Set a custom user agent string for http checks
2 participants