fix: ignore empty labels during label merge and templating #14322

ivankatliarchuk · 2022-02-20T14:03:53Z

Changes

When any string in label or andLabel is empty, Github API failes with Response code 422 (Unprocessable Entity). The fix should improve label API resiliency, as it possible, that during runtime some labels may not be set, or empty.

The code

return [...new Set([...labels, ...addLabels])]
      template.compile(label, config)
     .filter((el) => is.nonEmptyString(el) && !is.emptyStringOrWhitespace(el))
    .map((label) => template.compile(label, config))
    .filter((el) => is.nonEmptyString(el) && !is.emptyStringOrWhitespace(el));
  }
}

The first filter makes sure that input labels are not empty, not nuls and etc. In order to provide flexible configuration, Renovate supports using "templates", and when the value is not exist, not set or does not apply for a use case, handlebars return an empty string. Github API does not let to set empty label.

Context

Describe the bug

Any of depType , datasource or updateType empty at execution time.

module.exports = { 
  "labels": ["renovate"] ,
  "packageRules": [
    {
      "matchUpdateTypes": ["major", "minor", "patch", "pin", "digest"],
      "addLabels": ["{{depType}}", "{{datasource}}", "{{updateType}}"]
    },
  ]
}

or

Environment variable OPTIONAL_TEAM_LABEL is optional for team B but mandatory for team A

module.exports = { 
  "labels": [process.env.OPTIONAL_TEAM_LABEL] ,
}

Relevant debug logs

DEBUG: Adding labels 'renovate, , helm, major' to #7 (repository=ik-workshop/workshop-renovate, branch=renovate/sandbox/ex6-datadog-2.x)
DEBUG: Received invalid response - aborting (repository=ik-workshop/workshop-renovate, branch=renovate/sandbox/ex6-datadog-2.x)
       "err": {
         "name": "HTTPError",
         "code": "ERR_NON_2XX_3XX_RESPONSE",
         "timings": {
           "start": 1644788915104,
           "socket": 1644788915105,
           "lookup": 1644788915107,
           "connect": 1644788915138,
           "secureConnect": 1644788915169,
           "upload": 1644788915169,
           "response": 1644788915473,
           "end": 1644788915473,
           "phases": {
             "wait": 1,
             "dns": 2,
             "tcp": 31,
             "tls": 31,
             "request": 0,
             "firstByte": 304,
             "download": 0,
             "total": 369
           }
         },
         "message": "Response code 422 (Unprocessable Entity)",
         "stack": "HTTPError: Response code 422 (Unprocessable Entity)\n    at Request.<anonymous> (/usr/src/app/node_modules/got/dist/source/as-promise/index.js:117:42)\n    at processTicksAndRejections (internal/process/task_queues.js:95:5)",
         "options": {
           "headers": {
             "user-agent": "RenovateBot/31.72.2 (https://github.com/renovatebot/renovate)",
             "accept": "application/vnd.github.v3+json",
             "authorization": "***********",
             "content-type": "application/json",
             "content-length": "30",
             "accept-encoding": "gzip, deflate, br"
           },
           "url": "https://api.github.com/repos/ik-workshop/workshop-renovate/issues/7/labels",
           "hostType": "github",
           "username": "",
           "password": "",
           "method": "POST",
           "http2": false
         },
         "response": {
           "statusCode": 422,
           "statusMessage": "Unprocessable Entity",
           "body": {
             "message": "Validation Failed",
             "errors": [
               {
                 "value": "major",
                 "resource": "Label",
                 "field": "name",
                 "code": "invalid"
               }
             ],
             "documentation_url": "https://docs.github.com/rest/reference/issues#add-labels-to-an-issue"
           },
           "headers": {
             "server": "GitHub.com",
             "date": "Sun, 13 Feb 2022 21:48:35 GMT",
             "content-type": "application/json; charset=utf-8",
             "content-length": "202",
             "x-oauth-scopes": "repo, workflow",
             "x-accepted-oauth-scopes": "",
             "github-authentication-token-expiration": "2022-03-12 09:39:59 UTC",
             "x-github-media-type": "github.v3; format=json",
             "x-ratelimit-limit": "5000",
             "x-ratelimit-remaining": "4936",
             "x-ratelimit-reset": "1644791050",
             "x-ratelimit-used": "64",
             "x-ratelimit-resource": "core",
             "access-control-expose-headers": "ETag, Link, Location, Retry-After, X-GitHub-OTP, X-RateLimit-Limit, X-RateLimit-Remaining, X-RateLimit-Used, X-RateLimit-Resource, X-RateLimit-Reset, X-OAuth-Scopes, X-Accepted-OAuth-Scopes, X-Poll-Interval, X-GitHub-Media-Type, X-GitHub-SSO, X-GitHub-Request-Id, Deprecation, Sunset",
             "access-control-allow-origin": "*",
             "strict-transport-security": "max-age=31536000; includeSubdomains; preload",
             "x-frame-options": "deny",
             "x-content-type-options": "nosniff",
             "x-xss-protection": "0",
             "referrer-policy": "origin-when-cross-origin, strict-origin-when-cross-origin",
             "content-security-policy": "default-src 'none'",
             "vary": "Accept-Encoding, Accept, X-Requested-With",
             "x-github-request-id": "D677:DFBB:20D48F1:216B744:62097CB3",
             "connection": "close"
           },
           "httpVersion": "1.1"
         }
       }
DEBUG: Pull request creation error (repository=ik-workshop/workshop-renovate, branch=renovate/sandbox/ex6-datadog-2.x)
       "err": {
         "message": "repository-changed",
         "stack": "Error: repository-changed\n    at handleGotError (/usr/src/app/node_modules/renovate/lib/util/http/github.ts:132:14)\n    at GithubHttp.request (/usr/src/app/node_modules/renovate/lib/util/http/github.ts:329:13)\n    at processTicksAndRejections (internal/process/task_queues.js:95:5)\n    at GithubHttp.requestJson (/usr/src/app/node_modules/renovate/lib/util/http/index.ts:238:17)\n    at addLabels (/usr/src/app/node_modules/renovate/lib/platform/github/index.ts:1259:5)\n    at Proxy.createPr (/usr/src/app/node_modules/renovate/lib/platform/github/index.ts:1518:3)\n    at ensurePr (/usr/src/app/node_modules/renovate/lib/workers/pr/index.ts:441:14)\n    at processBranch (/usr/src/app/node_modules/renovate/lib/workers/branch/index.ts:620:33)\n    at writeUpdates (/usr/src/app/node_modules/renovate/lib/workers/repository/process/write.ts:38:17)\n    at update (/usr/src/app/node_modules/renovate/lib/workers/repository/process/extract-update.ts:129:11)\n    at Object.renovateRepository (/usr/src/app/node_modules/renovate/lib/workers/repository/index.ts:46:17)\n    at Object.start (/usr/src/app/node_modules/renovate/lib/workers/global/index.ts:126:7)\n    at /usr/src/app/node_modules/renovate/lib/renovate.ts:16:22"
       }

Documentation (please check one with an [x])

I have updated the documentation, or
No documentation update is required

How I've tested my work (please tick one)

I have verified these changes via:

Code inspection only, or
Newly added/modified unit tests, or
No unit tests but ran on a real repository, or
Both unit tests + ran on a real repository

CLAassistant · 2022-02-20T14:03:58Z

All committers have signed the CLA.

rarkins

This change affects more than just "don't set empty labels" and could have unforeseen reason implications. Please either fix the narrow scope or else start an issue to discuss wider requirements.

I also recommend you simplify to use is.truthy and is.nonEmptyString

lib/util/template/index.ts

viceice · 2022-02-20T14:33:11Z

lib/workers/pr/index.ts

-    template.compile(label, config)
-  );
+  return [...new Set([...labels, ...addLabels])]
+    .filter((el) => el !== null && el !== undefined && el.trim() !== '')


Suggested change

.filter((el) => el !== null && el !== undefined && el.trim() !== '')

.filter(is.nonEmptyString)

Tests are failing. In this case, is.nonEmptyString does not cover a case when the string contains multiple white space characters.
It still require trim e.g. .filter((el) => is.nonEmptyString(el) && el.trim() !== '')

Thanks for help. Ive updated the logic with .filter((el) => is.nonEmptyString(el) && !is.emptyStringOrWhitespace(el)) and created a proposal to add nonEmptyStringOrWhitespace sindresorhus/is#158

viceice · 2022-02-20T14:33:40Z

lib/workers/pr/index.ts

+  return [...new Set([...labels, ...addLabels])]
+    .filter((el) => el !== null && el !== undefined && el.trim() !== '')
+    .map((label) => template.compile(label, config))
+    .filter((el) => el !== null && el.trim() !== '');


Suggested change

.filter((el) => el !== null && el.trim() !== '');

.filter(is.nonEmptyString);

updated with .filter((el) => is.nonEmptyString(el) && !is.emptyStringOrWhitespace(el))

Tested. Make sense Co-authored-by: Michael Kriese <michael.kriese@visualon.de>

ivankatliarchuk · 2022-02-20T21:41:32Z

Thanks for the suggestions. Can I have please an idea of how to reduce the scope for the bug described?
Created an issue #14328

HonkingGoose · 2022-02-21T10:35:43Z

Hi @ivankatliarchuk! 👋

Thank you for your contribution!

You can mark a PR as a "draft" from within GitHub. You don't need to put (draft) in the PR title. 😉

Please read the GitHub docs, Changing the stage of a pull request, and then mark the PR as draft within GitHub, and remove the (draft) text from the PR title.

HonkingGoose · 2022-02-21T11:45:39Z

@rarkins don't you want to edit the PR title first, before you merge?

viceice

Otherwise LGTM

lib/util/template/index.ts

lib/workers/pr/index.ts

ivankatliarchuk · 2022-02-21T13:56:26Z

Hi, @HonkingGoose. Apologies spent too much time working on GitLab. The Draft button there is a bit more visible. I found it btw -;)

Co-authored-by: Michael Kriese <michael.kriese@visualon.de>

lib/workers/pr/index.ts

renovate-release · 2022-02-21T21:09:24Z

🎉 This PR is included in version 31.89.7 🎉

The release is available on:

GitHub release
31.89.7

Your semantic-release bot 📦🚀

fix: do ignore empty git labels during label merge and templating

1f6d694

ivankatliarchuk changed the title ~~fix: do ignore empty git labels during label merge and templating~~ fix: do ignore empty git labels during label merge and templating (draft) Feb 20, 2022

rarkins requested changes Feb 20, 2022

View reviewed changes

viceice requested changes Feb 20, 2022

View reviewed changes

ivankatliarchuk and others added 2 commits February 20, 2022 14:44

Update lib/util/template/index.ts

6d89af7

Tested. Make sense Co-authored-by: Michael Kriese <michael.kriese@visualon.de>

fix: wrap filter login with is library functions

f7ea715

ivankatliarchuk mentioned this pull request Feb 20, 2022

Renovate fails to create or update labels when there are an empty labels #14328

Closed

Merge branch 'main' into fix/empty-labels

8ed8217

rarkins previously approved these changes Feb 21, 2022

View reviewed changes

rarkins enabled auto-merge (squash) February 21, 2022 11:41

rarkins changed the title ~~fix: do ignore empty git labels during label merge and templating (draft)~~ fix: do ignore empty git labels during label merge and templating Feb 21, 2022

viceice reviewed Feb 21, 2022

View reviewed changes

lib/util/template/index.ts Outdated Show resolved Hide resolved

lib/workers/pr/index.ts Outdated Show resolved Hide resolved

Update lib/util/template/index.ts

71e5571

Co-authored-by: Michael Kriese <michael.kriese@visualon.de>

rarkins dismissed their stale review via 71e5571 February 21, 2022 15:46

added helper function

7c26ade

auto-merge was automatically disabled February 21, 2022 19:26
Head branch was pushed to by a user without write access

ivankatliarchuk requested a review from viceice February 21, 2022 19:28

viceice reviewed Feb 21, 2022

View reviewed changes

lib/workers/pr/index.ts Outdated Show resolved Hide resolved

fix: move function into utils folder

42fe02a

viceice approved these changes Feb 21, 2022

View reviewed changes

Merge branch 'main' into fix/empty-labels

9102f9b

viceice changed the title ~~fix: do ignore empty git labels during label merge and templating~~ fix: ignore empty labels during label merge and templating Feb 21, 2022

viceice requested a review from rarkins February 21, 2022 20:29

rarkins merged commit 9a103cb into renovatebot:main Feb 21, 2022

ivankatliarchuk deleted the fix/empty-labels branch February 21, 2022 20:50

github-actions bot locked as resolved and limited conversation to collaborators Mar 24, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: ignore empty labels during label merge and templating #14322

fix: ignore empty labels during label merge and templating #14322

ivankatliarchuk commented Feb 20, 2022 •

edited

Loading

CLAassistant commented Feb 20, 2022 •

edited

Loading

rarkins left a comment

viceice Feb 20, 2022

ivankatliarchuk Feb 20, 2022 •

edited

Loading

ivankatliarchuk Feb 20, 2022

viceice Feb 20, 2022

ivankatliarchuk Feb 20, 2022

ivankatliarchuk commented Feb 20, 2022

HonkingGoose commented Feb 21, 2022

HonkingGoose commented Feb 21, 2022

viceice left a comment

ivankatliarchuk commented Feb 21, 2022

renovate-release commented Feb 21, 2022

	.filter((el) => el !== null && el !== undefined && el.trim() !== '')
	.filter(is.nonEmptyString)

	.filter((el) => el !== null && el.trim() !== '');
	.filter(is.nonEmptyString);

fix: ignore empty labels during label merge and templating #14322

fix: ignore empty labels during label merge and templating #14322

Conversation

ivankatliarchuk commented Feb 20, 2022 • edited Loading

Changes

Context

Documentation (please check one with an [x])

How I've tested my work (please tick one)

CLAassistant commented Feb 20, 2022 • edited Loading

rarkins left a comment

Choose a reason for hiding this comment

viceice Feb 20, 2022

Choose a reason for hiding this comment

ivankatliarchuk Feb 20, 2022 • edited Loading

Choose a reason for hiding this comment

ivankatliarchuk Feb 20, 2022

Choose a reason for hiding this comment

viceice Feb 20, 2022

Choose a reason for hiding this comment

ivankatliarchuk Feb 20, 2022

Choose a reason for hiding this comment

ivankatliarchuk commented Feb 20, 2022

HonkingGoose commented Feb 21, 2022

HonkingGoose commented Feb 21, 2022

viceice left a comment

Choose a reason for hiding this comment

ivankatliarchuk commented Feb 21, 2022

renovate-release commented Feb 21, 2022

ivankatliarchuk commented Feb 20, 2022 •

edited

Loading

CLAassistant commented Feb 20, 2022 •

edited

Loading

ivankatliarchuk Feb 20, 2022 •

edited

Loading