Skip to content

Update versions of components for base image #7411

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

Merged
merged 2 commits into from
Aug 9, 2021

Conversation

iamNoah1
Copy link
Contributor

@iamNoah1 iamNoah1 commented Jul 30, 2021

Updating versions of components for our base image to the newest available versions.

Which issue/s this PR fixes

fixes #7320

How Has This Been Tested?

  • On a Ubuntu 18.04
  • Updated versions in images/nginx/rootfs/build.sh
  • Set OUTPUT= -o type=docker inside of images/nginx/Makefile in order to build a docker image in local registry
  • Set PLATFORMS?=linux/amd64 inside of images/nginx/Makefile to only build amd64 for the test run.
  • make build inside of images/nginx
  • Set BASE_IMAGE to newly created image inside of ingress-nginx/Makefile
  • Run make kind-e2e-test

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I've read the CONTRIBUTION guide
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Jul 30, 2021
@k8s-ci-robot
Copy link
Contributor

@iamNoah1: This issue is currently awaiting triage.

If Ingress contributors determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot added the needs-kind Indicates a PR lacks a `kind/foo` label and requires one. label Jul 30, 2021
@k8s-ci-robot
Copy link
Contributor

Hi @iamNoah1. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. needs-priority size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jul 30, 2021
@k8s-ci-robot k8s-ci-robot requested review from bowei and justinsb July 30, 2021 06:48
Copy link
Member

@tao12345666333 tao12345666333 left a comment

Choose a reason for hiding this comment

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

Thanks!
I'm not sure if they will have compatibility issues with each other

But let CI test it first 🚀

/ok-to-test

@k8s-ci-robot k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jul 30, 2021
@iamNoah1
Copy link
Contributor Author

/kind cleanup
/priority backlog
/triage accepted

@k8s-ci-robot k8s-ci-robot added kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. priority/backlog Higher priority than priority/awaiting-more-evidence. and removed needs-kind Indicates a PR lacks a `kind/foo` label and requires one. needs-priority labels Jul 30, 2021
@k8s-ci-robot
Copy link
Contributor

@iamNoah1: The label triage/accepted cannot be applied. Only GitHub organization members can add the label.

In response to this:

/kind cleanup
/priority backlog
/triage accepted

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

export NGINX_SUBSTITUTIONS=bc58cb11844bc42735bbaef7085ea86ace46d05b
export NGINX_OPENTRACING_VERSION=0.11.0
export OPENTRACING_CPP_VERSION=1.6.0
export SETMISC_VERSION=fddc347134e97d22c34dc5189e76b0ec97bf84d7
Copy link
Contributor

Choose a reason for hiding this comment

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

ok, as there's no tag cut since 2018

Copy link
Contributor

Choose a reason for hiding this comment

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

EDIT: openresty/set-misc-nginx-module@v0.32...master

I don't think here it's worth the effort to change, no bugfix since there :)

export NGINX_OPENTRACING_VERSION=0.11.0
export OPENTRACING_CPP_VERSION=1.6.0
export SETMISC_VERSION=fddc347134e97d22c34dc5189e76b0ec97bf84d7
export MORE_HEADERS_VERSION=f85af9649b858e21b400a2150a4c7b8ebd36e921
Copy link
Contributor

Choose a reason for hiding this comment

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

ok, as there's no tag cut since 2017

Copy link
Contributor

Choose a reason for hiding this comment

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

EDIT: same: openresty/headers-more-nginx-module@v0.33...master

No big changes since the last release

export OPENTRACING_CPP_VERSION=1.6.0
export SETMISC_VERSION=fddc347134e97d22c34dc5189e76b0ec97bf84d7
export MORE_HEADERS_VERSION=f85af9649b858e21b400a2150a4c7b8ebd36e921
export NGINX_DIGEST_AUTH=1.0.0
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

export MORE_HEADERS_VERSION=f85af9649b858e21b400a2150a4c7b8ebd36e921
export NGINX_DIGEST_AUTH=1.0.0
export NGINX_SUBSTITUTIONS=b8a71eacc7f986ba091282ab8b1bbbc6ae1807e0
export NGINX_OPENTRACING_VERSION=2d9f40fa86380b019698f131a803f415ad24c033
Copy link
Contributor

Choose a reason for hiding this comment

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

@iamNoah1 let's use 0.19.0 here

export LUA_STREAM_NGX_VERSION=0.0.9
export LUA_UPSTREAM_VERSION=0.07
export LUA_CJSON_VERSION=2.1.0.8
export YAML_CPP_VERSION=b591d8ae2ad1ff373273c3e05973adf6c46abfa8
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's use yaml-cpp-0.7.0

export LUA_UPSTREAM_VERSION=0.07
export LUA_CJSON_VERSION=2.1.0.8
export YAML_CPP_VERSION=b591d8ae2ad1ff373273c3e05973adf6c46abfa8
export JAEGER_VERSION=e90642a250289623fc13f650991d108e9a19b6b1
Copy link
Contributor

Choose a reason for hiding this comment

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

I think here as Jaeger got a pretty "recent" release we can stick with 0.7.0

Copy link
Contributor

Choose a reason for hiding this comment

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

(also the recent commits where to change their issue template

Comment on lines 37 to 38
export LUA_NGX_VERSION=66a4d417d654404ade32fcc600ff1381e5e02f9c
export LUA_STREAM_NGX_VERSION=b977cee61a1cc0ea988a581cf0adbc955c9a95ab
Copy link
Contributor

Choose a reason for hiding this comment

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

For both of those, there has been some recent bugfixes, maybe we should move to the last commit


export LUAJIT_VERSION=2.1-20210510

export LUA_RESTY_BALANCER=2e1848e729eb8558d533ce97ffdf61edead39953
Copy link
Contributor

Choose a reason for hiding this comment

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

This one got a recent bugfix about balancer weight, probably impacts on us

export LUAJIT_VERSION=2.1-20210510

export LUA_RESTY_BALANCER=2e1848e729eb8558d533ce97ffdf61edead39953
export LUA_RESTY_CACHE=dacd022b4b8e3a73ed25886f2f9db043075ddb31
Copy link
Contributor

Choose a reason for hiding this comment

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

We can use 0.11 here (as the difference between version bump and this commit is just the maintainer dropping travis-ci usage)


export LUA_RESTY_BALANCER=2e1848e729eb8558d533ce97ffdf61edead39953
export LUA_RESTY_CACHE=dacd022b4b8e3a73ed25886f2f9db043075ddb31
export LUA_RESTY_CORE=b8a4b2cfe35bcb65c2ada52c17503ece7cd3288a
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's use 0.1.22 here

export LUA_RESTY_COOKIE_VERSION=303e32e512defced053a6484bc0745cf9dc0d39e
export LUA_RESTY_DNS=0.22
export LUA_RESTY_HTTP=0.16.1
export LUA_RESTY_LOCK=ad94745a4731ee6ddbffbc3b602df25ca7c6f7ba
Copy link
Contributor

Choose a reason for hiding this comment

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

We can keep 0.08 here (openresty/lua-resty-lock@v0.08...master) no changes :)

export LUA_RESTY_DNS=0.22
export LUA_RESTY_HTTP=0.16.1
export LUA_RESTY_LOCK=ad94745a4731ee6ddbffbc3b602df25ca7c6f7ba
export LUA_RESTY_UPLOAD_VERSION=7baca92c7e741979ae5857989bbf6cc0402d6126
Copy link
Contributor

Choose a reason for hiding this comment

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

export LUA_RESTY_LOCK=ad94745a4731ee6ddbffbc3b602df25ca7c6f7ba
export LUA_RESTY_UPLOAD_VERSION=7baca92c7e741979ae5857989bbf6cc0402d6126
export LUA_RESTY_STRING_VERSION=9ace36f2dde09451c377c839117ade45eb02d460
export LUA_RESTY_MEMCACHED_VERSION=1517409a3f23116bb63f080629a6a515cf332d87
Copy link
Contributor

Choose a reason for hiding this comment

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

export LUA_RESTY_UPLOAD_VERSION=7baca92c7e741979ae5857989bbf6cc0402d6126
export LUA_RESTY_STRING_VERSION=9ace36f2dde09451c377c839117ade45eb02d460
export LUA_RESTY_MEMCACHED_VERSION=1517409a3f23116bb63f080629a6a515cf332d87
export LUA_RESTY_REDIS_VERSION=91585affcd9a8da65cb664a5b1e926dde428095a
Copy link
Contributor

Choose a reason for hiding this comment

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

Not necessary to change, no big change: openresty/lua-resty-redis@v0.29...master

Copy link
Contributor

@rikatz rikatz left a comment

Choose a reason for hiding this comment

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

@iamNoah1 thanks, this is huge!

Left some comments, in three categories:

  • Lines that does not needs changes (last releases were nothing big), with branch comparision
  • Lines that we should stick with release version instead of going to a commit
  • Lines that got a commit since you've sent the PR and I think it would be good to move to the latest commit

Also, as proposed in Slack, if you've got time/patience for this, let's add in front of things that got no recent release (like SETMISC_VERSION and MORE_HEADERS_VERSION) a comment saying that this got no recent release, and that it should be verified with branch comparision, for example:

# MORE_HEADERS_VERSION got no recent change, see: https://github.com/openresty/headers-more-nginx-module/compare/v0.33...master
export MORE_HEADERS_VERSION=0.33 

Once again, thanks for this!

@iamNoah1
Copy link
Contributor Author

iamNoah1 commented Aug 3, 2021

Next steps: Fix proposals, make e2e on a machine. Merge PR. Follow up with release.md process.

@rikatz
Copy link
Contributor

rikatz commented Aug 5, 2021

Next steps: Fix proposals, make e2e on a machine. Merge PR. Follow up with release.md process.

Sounds good! Ping me when this is ready (on Slack) as I'm missing a lot of GH notifications :/

Want to get this merged before the (proposed) code freeze :)

export DATADOG_CPP_VERSION=5e3e3571d2341883a15fc4b5b443f930cc70e8c1
export MODSECURITY_VERSION=2497e6ac654d0b117b9534aa735b757c6b11c84f
export MODSECURITY_LIB_VERSION=v3.0.5
export OWASP_MODSECURITY_CRS_VERSION=v3.3.2
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This one is somehow strange. Main Branch seems to be v3.4/dev. But when comparing with tag v3.3.2 the changes are way too big(coreruleset/coreruleset@v3.3.2...v3.4/dev). I don't get it. @rikatz can you maybe doubelcheck.

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah, and in this one I have some knowledge :)

So mod_security on dev branch renames a lot of stuff. This one is specifically the ruleset for modsec. I wouldn't use the dev branch, as rules may be unstable and break users

export GEOIP2_VERSION=a26c6beed77e81553686852dceb6c7fdacc5970d
export NGINX_AJP_VERSION=a964a0bcc6a9f2bfb82a13752d7794a36319ffac

export LUAJIT_VERSION=2.1-20210510
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Whats with this one @rikatz? openresty/luajit2@v2.1-20210510...v2.1-agentzh shows that has been a lot going on since the last release.

Copy link
Contributor

Choose a reason for hiding this comment

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

Does not seem to be an official release :(

https://github.com/openresty/luajit2/releases

This is why I've pointed to the release

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I kept using the 2.1-20210510. But v2.1-agentzh seems to be the main branch. So after release 2.1-20210510 there was some commits going on. Or am I wrong?!.

Copy link
Contributor

Choose a reason for hiding this comment

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

you are right :) I just wanted to stick with the releases specifically for luajit2 due to its critical role :D

@iamNoah1 iamNoah1 force-pushed the update-images branch 3 times, most recently from 9800777 to a227449 Compare August 8, 2021 08:23
@rikatz
Copy link
Contributor

rikatz commented Aug 9, 2021

/label tide/merge-method-squash
/lgtm
/approve
@iamNoah1 thank you very much for this!! Let's wait until the cloudbuild finishes (~2 hours...), promote the image and change the base image on all the branches! :D

@k8s-ci-robot k8s-ci-robot added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Aug 9, 2021
@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 9, 2021
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: iamNoah1, rikatz

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 9, 2021
@k8s-ci-robot k8s-ci-robot merged commit 98288bc into kubernetes:main Aug 9, 2021
@iamNoah1
Copy link
Contributor Author

iamNoah1 commented Aug 9, 2021

/label tide/merge-method-squash
/lgtm
/approve
@iamNoah1 thank you very much for this!! Let's wait until the cloudbuild finishes (~2 hours...), promote the image and change the base image on all the branches! :D

Updating the image on all branches will be fun ^^

rchshld pushed a commit to joomcode/ingress-nginx that referenced this pull request May 19, 2023
* update versions and checksums

* change requests from PR
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. priority/backlog Higher priority than priority/awaiting-more-evidence. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update image versions
4 participants