Skip to content

fix(ui): Update default and max count for maxCookieNumber#14898

Closed
zvlb wants to merge 17 commits intoargoproj:masterfrom
kaasops:update-cookies-number
Closed

fix(ui): Update default and max count for maxCookieNumber#14898
zvlb wants to merge 17 commits intoargoproj:masterfrom
kaasops:update-cookies-number

Conversation

@zvlb
Copy link
Contributor

@zvlb zvlb commented Aug 4, 2023

In this PR I update default and max count for variable - maxCookieNumber

Default switch from 10 to 20, bc in RFC 2109 recommend:

      * at least 20 cookies per unique host or domain name

And switch max Cookies number from 30 to math.MaxInt64, bc:

  • RFC 2109 don't have recomenration about MAX cookies count, only MIN. Ideally recommend have no limits on the number of cookies.
  • I'm, as a ArgoCD administrator, want to be able to control limits for cookie.

@crenshaw-dev
Copy link
Member

@zvlb can you rebase? There's a flaky test that's fixed on the latest commit.

@zvlb
Copy link
Contributor Author

zvlb commented Aug 8, 2023

@zvlb can you rebase? There's a flaky test that's fixed on the latest commit.

@crenshaw-dev
I did it, but in integrations tests failed(

Copy link
Member

@ishitasequeira ishitasequeira left a comment

Choose a reason for hiding this comment

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

@zvlb , It looks like the test TestCookieMaxLength is failing. You would need to update the message here to below for passing the test.

the authentication token is 81760 characters long and requires 21 cookies but the max number of cookies is 20. Contact your Argo CD administrator to increase the max number of cookies

@codecov
Copy link

codecov bot commented Aug 9, 2023

Codecov Report

Patch coverage: 33.82% and project coverage change: -0.01% ⚠️

Comparison is base (8270225) 49.91% compared to head (465ab5d) 49.91%.
Report is 16 commits behind head on master.

❗ Current head 465ab5d differs from pull request most recent head 18906e2. Consider uploading reports for the commit 18906e2 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #14898      +/-   ##
==========================================
- Coverage   49.91%   49.91%   -0.01%     
==========================================
  Files         262      262              
  Lines       44952    45002      +50     
==========================================
+ Hits        22440    22464      +24     
- Misses      20307    20329      +22     
- Partials     2205     2209       +4     
Files Changed Coverage Δ
...cationset/controllers/applicationset_controller.go 62.64% <0.00%> (ø)
applicationset/generators/duck_type.go 70.18% <0.00%> (ø)
...licationset/generators/generator_spec_processor.go 59.63% <0.00%> (ø)
applicationset/generators/merge.go 52.40% <0.00%> (ø)
applicationset/generators/plugin.go 75.52% <0.00%> (ø)
applicationset/generators/scm_provider.go 43.21% <0.00%> (ø)
controller/cache/cache.go 27.01% <0.00%> (ø)
controller/state.go 71.33% <0.00%> (-1.08%) ⬇️
util/db/helmrepository.go 0.00% <0.00%> (ø)
util/db/repository_legacy.go 46.77% <0.00%> (ø)
... and 14 more

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

zvlb and others added 17 commits August 9, 2023 15:26
Signed-off-by: zvlb <vl.zemtsov@gmail.com>
* fix: Fixes health icon positioning argoproj#14708

Signed-off-by: ashinsabu3 <ashin.sabu@harness.io>

* fix: Fixes alignment of app health application status panel argoproj#14708

Signed-off-by: ashinsabu3 <ashin.sabu@harness.io>

* fix: Added line height to App Status to fix its  positioning argoproj#14708

Signed-off-by: ashinsabu3 <ashin.sabu@harness.io>

---------

Signed-off-by: ashinsabu3 <ashin.sabu@harness.io>
Signed-off-by: zvlb <vl.zemtsov@gmail.com>
Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Signed-off-by: zvlb <vl.zemtsov@gmail.com>
…when reading tags from OCI registry (argoproj#14537)

* fix: Change underscore (_) back to plus (+) to get valid SemVer when reading tags from OCI registry

Signed-off-by: xashr <saschasynaos@gmail.com>

* Add test coverage for SemVer tags in TestGetTagsFromUrl

Signed-off-by: xashr <saschasynaos@gmail.com>

---------

Signed-off-by: xashr <saschasynaos@gmail.com>
Signed-off-by: zvlb <vl.zemtsov@gmail.com>
…VIDERS (argoproj#14902) (argoproj#14913)

Signed-off-by: gmuselli <geoffrey.muselli@gmail.com>
Signed-off-by: zvlb <vl.zemtsov@gmail.com>
argoproj#14926)

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>
Signed-off-by: zvlb <vl.zemtsov@gmail.com>
…argoproj#14925)

Bumps [github.com/aws/aws-sdk-go](https://github.com/aws/aws-sdk-go) from 1.44.312 to 1.44.317.
- [Release notes](https://github.com/aws/aws-sdk-go/releases)
- [Commits](aws/aws-sdk-go@v1.44.312...v1.44.317)

---
updated-dependencies:
- dependency-name: github.com/aws/aws-sdk-go
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: zvlb <vl.zemtsov@gmail.com>
…#14922)

Bumps [golang.org/x/oauth2](https://github.com/golang/oauth2) from 0.10.0 to 0.11.0.
- [Commits](golang/oauth2@v0.10.0...v0.11.0)

---
updated-dependencies:
- dependency-name: golang.org/x/oauth2
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: zvlb <vl.zemtsov@gmail.com>
Remove a misleading symbol from the pattern for the path.Match function. The pipe symbol doesn't have any special meaning.

Signed-off-by: German Lashevich <german.lashevich@gmail.com>
Signed-off-by: zvlb <vl.zemtsov@gmail.com>
…ockerfile blocks (argoproj#14911)

Signed-off-by: JesseBot <jessebot@linux.com>
Signed-off-by: zvlb <vl.zemtsov@gmail.com>
argoproj#5117) (argoproj#14917)

Signed-off-by: Vipin M S <vipinachar2016@gmail.com>
Signed-off-by: zvlb <vl.zemtsov@gmail.com>
Signed-off-by: CI <ci@argoproj.com>
Co-authored-by: CI <ci@argoproj.com>
Signed-off-by: zvlb <vl.zemtsov@gmail.com>
* chore: wrap error objects to include context

Signed-off-by: Vipin M S <vipinachar2016@gmail.com>

* chore: wrap error objects to include context

Signed-off-by: Vipin M S <vipinachar2016@gmail.com>

* chore: wrap error objects to include context

Signed-off-by: Vipin M S <vipinachar2016@gmail.com>

* chore: wrap error objects to include context

Signed-off-by: Vipin M S <vipinachar2016@gmail.com>

* Update applicationset/controllers/applicationset_controller.go

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>

---------

Signed-off-by: Vipin M S <vipinachar2016@gmail.com>
Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Co-authored-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Signed-off-by: zvlb <vl.zemtsov@gmail.com>
Signed-off-by: zvlb <vl.zemtsov@gmail.com>
…shared projects from subgroups projects (argoproj#14831)

* added option to disable gitlab to fetch shared project from a subgroup

Signed-off-by: Prune <prune@lecentre.net>

* Correct gitlab SCM provider mock test

Signed-off-by: Prune <prune@lecentre.net>

* updated test to validate shared-groups

Signed-off-by: Prune <prune@lecentre.net>

* reworked shared project tests

Signed-off-by: Prune <prune@lecentre.net>

* added subgroups only test

Signed-off-by: Prune <prune@lecentre.net>

---------

Signed-off-by: Prune <prune@lecentre.net>
Signed-off-by: zvlb <vl.zemtsov@gmail.com>
…tential cleanup (argoproj#9180) (argoproj#14955)

* fix: send sigterm to cmp commands before sigkill to allow for potential cleanup

Signed-off-by: Ashin Sabu <ashin.sabu@harness.io>

* fix: unit test for runCommand in cmpserver to test cleanup modified

Signed-off-by: Ashin Sabu <ashin.sabu@harness.io>

* fix: change unit test for plugin/runCommand to avoid bad trap along with lint fix

Signed-off-by: Ashin Sabu <ashin.sabu@harness.io>

---------

Signed-off-by: Ashin Sabu <ashin.sabu@harness.io>
Signed-off-by: zvlb <vl.zemtsov@gmail.com>
Signed-off-by: zvlb <vl.zemtsov@gmail.com>
@zvlb
Copy link
Contributor Author

zvlb commented Aug 9, 2023

Oh. Sorry, I broke this branch) I don't wont fix it. I creare new PR without problem - 14979

@zvlb zvlb closed this Aug 9, 2023
@zvlb zvlb deleted the update-cookies-number branch August 9, 2023 12:38
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.