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.

Sign up for GitHub

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

Jump to bottom

Introduce disabledAPIVersions #2951

Merged
merged 14 commits into from
Jan 6, 2023
Merged

Introduce disabledAPIVersions #2951

merged 14 commits into from
Jan 6, 2023

Conversation

pcapriotti
Copy link
Contributor

@pcapriotti pcapriotti commented Dec 23, 2022 •
edited by smatting
Loading

This PR adds a disabledAPIVersions option to all services, which can be used to disable certain versions.
See also changes to the documention in this PR.

https://wearezeta.atlassian.net/browse/FS-1046

Checklist

  • Add a new entry in an appropriate subdirectory of changelog.d
  • Read and follow the PR guidelines

Additional checks

To test the helm charts disable V2 on all services then call a v2 endpoint and assert that response is 404 with a unsupported-version tag.

- [x] brig
http GET http://localhost:8080/v2/self Z-User:c64d2f82-9573-41aa-b8bd-b6c7da87540d

- [x] cannon
http GET http://localhost:8080/v2/await Z-User:c64d2f82-9573-41aa-b8bd-b6c7da87540d Z-Connection:deadbeef

- [x] cargohold
http POST http://localhost:8080/v2/assets/the-asset-key/token Z-User:c64d2f82-9573-41aa-b8bd-b6c7da87540d

- [x] galley
http POST http://localhost:8080/v2/teams

- [x] gundeck
http POST http://localhost:8080/v2/push/tokens Z-User:c64d2f82-9573-41aa-b8bd-b6c7da87540d Z-Connection:deadbeef foo=bar

- [x] proxy
http GET http://localhost:8080/v2/proxy/soundcloud/resolve

- [x] spar
http GET http://localhost:8080/v2/sso/settings

@pcapriotti pcapriotti temporarily deployed to cachix December 23, 2022 09:43 — with GitHub Actions Inactive
@pcapriotti pcapriotti temporarily deployed to cachix December 23, 2022 09:43 — with GitHub Actions Inactive
@zebot zebot added the ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist label Dec 23, 2022
@pcapriotti pcapriotti temporarily deployed to cachix December 23, 2022 12:27 — with GitHub Actions Inactive
@pcapriotti pcapriotti temporarily deployed to cachix December 23, 2022 12:27 — with GitHub Actions Inactive
@pcapriotti pcapriotti force-pushed the pcapriotti/api-config branch from a475c8a to d7f2f21 Compare December 23, 2022 12:57
@pcapriotti pcapriotti temporarily deployed to cachix December 23, 2022 12:57 — with GitHub Actions Inactive
@pcapriotti pcapriotti temporarily deployed to cachix December 23, 2022 12:57 — with GitHub Actions Inactive
@smatting smatting force-pushed the pcapriotti/api-config branch from d7f2f21 to ca479c8 Compare January 3, 2023 14:54
@smatting smatting temporarily deployed to cachix January 3, 2023 14:54 — with GitHub Actions Inactive
@smatting smatting temporarily deployed to cachix January 3, 2023 14:54 — with GitHub Actions Inactive
@smatting smatting changed the title Introduce API version blacklist Introduce API version denylist Jan 3, 2023
@smatting smatting changed the title Introduce API version denylist Introduce API version disabledVersions Jan 3, 2023
@smatting smatting changed the title Introduce API version disabledVersions Introduce disabledAPIVersions Jan 3, 2023
@smatting smatting force-pushed the pcapriotti/api-config branch from ca479c8 to 3252d48 Compare January 3, 2023 15:48
@smatting smatting temporarily deployed to cachix January 3, 2023 15:48 — with GitHub Actions Inactive
@smatting smatting temporarily deployed to cachix January 3, 2023 15:48 — with GitHub Actions Inactive
@smatting smatting temporarily deployed to cachix January 4, 2023 09:38 — with GitHub Actions Inactive
@smatting smatting temporarily deployed to cachix January 4, 2023 09:38 — with GitHub Actions Inactive
@smatting
Fix bug in cargho helm chart
33d3b65 
If .Values.config.settings is not set then the template will render to
empty string causing runtime problems. Federation domain is a required
paramter. The change in this commit turns the runtime error into a error
at template evaluation.
@smatting
Fix bug in helmfile-single
e4cc30b 
Federation domain is a required parameter for carghold
@smatting smatting temporarily deployed to cachix January 4, 2023 16:41 — with GitHub Actions Inactive
@smatting smatting temporarily deployed to cachix January 4, 2023 16:41 — with GitHub Actions Inactive
@smatting smatting force-pushed the pcapriotti/api-config branch from a5d397a to 3a78d69 Compare January 5, 2023 09:39
@smatting smatting temporarily deployed to cachix January 5, 2023 09:39 — with GitHub Actions Inactive
@smatting smatting temporarily deployed to cachix January 5, 2023 09:39 — with GitHub Actions Inactive
@smatting smatting marked this pull request as ready for review January 5, 2023 09:40
smatting
smatting reviewed Jan 5, 2023
View reviewed changes
charts/cargohold/templates/configmap.yaml
Comment on lines +44 to +46
{{- if .maxTotalBytes }}
maxTotalBytes: {{ .maxTotalBytes }}
{{- end }}
Copy link
Contributor

Choose a reason for hiding this comment

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

This causes this change in the configmap:
before:

maxTotalBytes: 5368709120

after

maxTotalBytes: 5.36870912e+09

This has no effect on the value read by the options parser howerver

@smatting smatting self-requested a review January 5, 2023 10:25
smatting
smatting reviewed Jan 5, 2023
View reviewed changes
charts/spar/templates/configmap.yaml
Comment on lines -46 to 51
{{ toYaml .contacts | indent 12 }}
{{- toYaml .contacts | nindent 8 }}
{{- end }}
Copy link
Contributor

Choose a reason for hiding this comment

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

I checked the rendere configmap. It is indented correctly.

@smatting smatting temporarily deployed to cachix January 5, 2023 10:30 — with GitHub Actions Inactive
@smatting smatting temporarily deployed to cachix January 5, 2023 10:30 — with GitHub Actions Inactive
mdimjasevic
mdimjasevic approved these changes Jan 5, 2023
View reviewed changes
Copy link
Contributor

@mdimjasevic mdimjasevic left a comment

Choose a reason for hiding this comment

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

Looks great!

docs/src/developer/developer/how-to.md Outdated Show resolved Hide resolved
@smatting @mdimjasevic
Update docs/src/developer/developer/how-to.md
1c3c829 
Co-authored-by: Marko Dimjašević <marko.dimjasevic@wire.com>
@smatting smatting temporarily deployed to cachix January 6, 2023 09:23 — with GitHub Actions Inactive
@smatting smatting temporarily deployed to cachix January 6, 2023 09:23 — with GitHub Actions Inactive
@smatting smatting merged commit b83b584 into develop Jan 6, 2023
@smatting smatting deleted the pcapriotti/api-config branch January 6, 2023 09:24
@zebot zebot mentioned this pull request Jan 12, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mdimjasevic mdimjasevic mdimjasevic approved these changes

@smatting smatting smatting approved these changes

Assignees
No one assigned
Labels
ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@pcapriotti @smatting @mdimjasevic @zebot