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

feat(api): add POST method that will add a uuid #8

Merged
merged 9 commits into from
Feb 1, 2024

Conversation

jasongoodwin
Copy link

(Intended to be published at 0.2.0).

This adds a POST method that will append /$UUID to the path.
It vendorizes the google uuid plugin as required by traefik.
Not sure if it works (how do I test it?), looks okay tho! We added it to Yoel's repo.

@jasongoodwin
Copy link
Author

Francis made a good observation that the generated uuid isn't returned to the client. Looking.

@jasongoodwin
Copy link
Author

Okay added a header that will return the ObjectLocation. Need to figure out how to test it - we can deploy/test by hand but if any guidance lmk.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
francisf2
francisf2 previously approved these changes Jan 31, 2024
@jasongoodwin
Copy link
Author

jasongoodwin commented Jan 31, 2024

Linting fails on this, can't reproduce locally. Looking...

Here is from my local run.

% golangci-lint run -v
INFO [config_reader] Config search paths: [./ /Users/jgoodwin/Development/src/new-traefik-plugin/mine/traefik-aws-plugin /Users/jgoodwin/Development/src/new-traefik-plugin/mine /Users/jgoodwin/Development/src/new-traefik-plugin /Users/jgoodwin/Development/src /Users/jgoodwin/Development /Users/jgoodwin /Users /]
INFO [lintersdb] Active 6 linters: [errcheck gosimple govet ineffassign staticcheck unused]
INFO [loader] Go packages loading at mode 575 (types_sizes|compiled_files|deps|exports_file|imports|name|files) took 357.170458ms
INFO [runner/filename_unadjuster] Pre-built 0 adjustments in 1.086291ms
INFO [linters_context/goanalysis] analyzers took 0s with no stages
INFO [runner] processing took 1.208µs with stages: max_same_issues: 333ns, skip_dirs: 167ns, nolint: 166ns, cgo: 84ns, identifier_marker: 42ns, path_prettifier: 42ns, autogenerated_exclude: 42ns, max_from_linter: 42ns, exclude: 42ns, max_per_file_from_linter: 42ns, skip_files: 42ns, severity-rules: 41ns, fixer: 41ns, diff: 41ns, uniq_by_line: 41ns, path_shortener: 0s, exclude-rules: 0s, source_code: 0s, path_prefixer: 0s, sort_results: 0s, filename_unadjuster: 0s
INFO [runner] linters took 199.164583ms with stages: goanalysis_metalinter: 199.14375ms
INFO File cache stats: 0 entries of total size 0B
INFO Memory: 7 samples, avg is 28.2MB, max is 35.9MB
INFO Execution took 566.039667ms
jgoodwin@MP16JGOODWIN1 traefik-aws-plugin % echo $?
0
% golangci-lint --version
golangci-lint has version 1.55.2 built with go1.21.3 from e3c2265 on 2023-11-02T21:40:02Z

It's different version local. Maybe will try updating.

@jasongoodwin
Copy link
Author

jasongoodwin commented Jan 31, 2024

I fixed the linting issue w/ the version changes but yaegi's testing fails to locate the vendored dependency (uuid)
Looks like the vendored packages have to be installed in the GOPATH for yaegi to pick them up for interpreting go.
traefik/yaegi#656 (comment)
Yoel and I were able to get it published from his repo after vendoring the plugin.

@jasongoodwin
Copy link
Author

jasongoodwin commented Jan 31, 2024

I checked what another plugin is doing - if you can approve this to get it running would appreciate it!
This is what I was looking at: https://github.com/wiltonsr/ldapAuth/blob/main/.github/workflows/main.yml
They just install/vendor the dependencies.

Also, I checked and Traefik is using go 1.21 in its pipeline. Hopefully no issue if we do too.
https://github.com/traefik/traefik/blob/master/.github/workflows/build.yaml

@@ -35,6 +35,14 @@ jobs:
- name: Set up GOPATH
run: go env -w GOPATH=${{ github.workspace }}/go


- name: Check and get dependencies
Copy link
Contributor

Choose a reason for hiding this comment

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

Offset seems wrong

Copy link
Author

Choose a reason for hiding this comment

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

Thanks - Fixed. was hoping to iterate on it on my fork first. Don't have admin access but I think there is a toggle to allow forks to run the workflow.

@yoeluk
Copy link

yoeluk commented Feb 1, 2024

LGTM

@yoeluk
Copy link

yoeluk commented Feb 1, 2024

@francisf2 @ilaidlaw can I have write access to this project?

@francisf2 francisf2 merged commit feef16f into bluecatengineering:main Feb 1, 2024
1 check passed
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.

3 participants