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: add github-actions tag support #7434

Merged
merged 17 commits into from
Oct 20, 2020

Conversation

RichiCoder1
Copy link
Contributor

Adds support for GitHub Action tag versions in GitHub Action Workflow Files.

Removes deprecated old-style workflow support.

Not include:

  • Git Sha Support

Closes #5733

Object {
"currentValue": "v1.0.0",
"datasource": "github-tags",
"depName": "actions/bin/shellcheck",
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think this is valid tags - perhaps needs lookupName? Also github.com/actions/bin doesn't exist either?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ha, that's fair. I'll update the examples to something more realistic and maybe add a path nesting limit.

Copy link
Collaborator

@rarkins rarkins Oct 9, 2020

Choose a reason for hiding this comment

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

if it's a/b/c then the a/b represents the repo while c is file or folder name? And if there's multiple actions within one repo then they share the same tags? Ie tags are per repo and not per action?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmmm. While actions technically supports repo subpaths, the recommendation is to have a single action per repo. At least for this first pass of getting this working, I wouldn't support the path'd version as the way most people would likely use that is via SHAs and not tags. Or would require some sort of tag prefixing support.

Copy link
Collaborator

Choose a reason for hiding this comment

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

If it's technically possible then we should ideally support it. In this case the lookupName would be a/b

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I actually did end up doing this. The latest version of the regex will extract owner/repo and ignore the path.

@RichiCoder1
Copy link
Contributor Author

RichiCoder1 commented Oct 9, 2020

Went ahead and improved the regex. It'll tolerate and ignore sub paths and tolerate more actual repo names, prefers loose versioning (which is more correct?), and fixes tag to be lookupName instead of depName, which appears to be what github-tags expects.

lib/manager/github-actions/extract.ts Outdated Show resolved Hide resolved
lib/manager/github-actions/extract.ts Outdated Show resolved Hide resolved
lib/manager/github-actions/extract.ts Outdated Show resolved Hide resolved
lib/manager/github-actions/extract.ts Outdated Show resolved Hide resolved
Comment on lines 50 to 51
depName: dep.depName,
currentValue: dep.currentValue,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
depName: dep.depName,
currentValue: dep.currentValue,
dep,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All the other examples were abbreviated, so wasn't sure. Changed

Copy link
Member

Choose a reason for hiding this comment

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

ah, i see now. some manager log the dep and some not. 🤔

@rarkins Should we pefer to remove the logging or should we add those logging to all managers? Maybe log at trace level?

Copy link
Collaborator

Choose a reason for hiding this comment

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

No, we ideally shouldn't have that in managers as it bloats the logs. We should convert them to trace

@RichiCoder1
Copy link
Contributor Author

Looking at other managers, should I specify depType?

@RichiCoder1 RichiCoder1 requested review from viceice and rarkins October 9, 2020 06:23
lib/manager/github-actions/extract.ts Outdated Show resolved Hide resolved
lib/manager/github-actions/extract.ts Outdated Show resolved Hide resolved
@viceice
Copy link
Member

viceice commented Oct 9, 2020

Looking at other managers, should I specify depType?

I don't think so.

lib/manager/github-actions/extract.ts Outdated Show resolved Hide resolved
lib/manager/github-actions/extract.ts Outdated Show resolved Hide resolved
lib/manager/github-actions/extract.ts Outdated Show resolved Hide resolved
@RichiCoder1
Copy link
Contributor Author

Switched to github-releases as the datasource, as this is more in line with what people expect and the marketplace. Per feedback, switched to docker versioning.

lib/manager/github-actions/extract.ts Outdated Show resolved Hide resolved
lib/manager/github-actions/extract.ts Outdated Show resolved Hide resolved
lib/manager/github-actions/extract.ts Show resolved Hide resolved
@rarkins
Copy link
Collaborator

rarkins commented Oct 11, 2020

@RichiCoder1 have you tested this latest code against any "real" repos?

@RichiCoder1
Copy link
Contributor Author

RichiCoder1 commented Oct 11, 2020

have you tested this latest code against any "real" repos?

I've been running a regex-ified version against two of my repos with no issues (using docker & github tags). Gotten some updates, no big issues so far. We have pretty minor, sane usage though.
Haven't had the chance (and not sure how best to test) this branch against a real repo.

@rarkins
Copy link
Collaborator

rarkins commented Oct 11, 2020

Let's make sure we have tested it against:

  • nested presets
  • short ones (eg v1)
  • regular semver
  • SHAs

Comment on lines 20 to 27
logger.debug(
{
depName: dep.depName,
currentValue: dep.currentValue,
currentDigest: dep.currentDigest,
},
'Docker image inside GitHub Actions'
'Docker image inside GitHub Workflow'
);
Copy link
Member

Choose a reason for hiding this comment

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

This should be removed

if (!dockerVersioning.api.isValid(currentValue)) {
dep.skipReason = SkipReason.InvalidVersion;
}
logger.trace(dep, 'GitHub Action inside GitHub Workflow');
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
logger.trace(dep, 'GitHub Action inside GitHub Workflow');

As we discussed, managers don't newed to log this, as all deps will be logged later anyways.

@rarkins
Copy link
Collaborator

rarkins commented Oct 20, 2020

I'll merge this once the current release is done

@rarkins rarkins merged commit 2374bca into renovatebot:master Oct 20, 2020
@renovate-release
Copy link
Collaborator

🎉 This PR is included in version 23.55.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@dargmuesli
Copy link

Will Renovate now start to update actions tags automatically or do I have to configure it to do so?

@viceice
Copy link
Member

viceice commented Oct 20, 2020

Will Renovate now start to update actions tags automatically or do I have to configure it to do so?

Yes, but the whitesource app is not yet updated, will be done today. So you shouldn't have to do anything.

@RichiCoder1
Copy link
Contributor Author

@rarkins 🥳 Glad I could help with this! Sorry I couldn't help more with testing

@RichiCoder1 RichiCoder1 deleted the feat/github-actions branch October 20, 2020 14:31
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support to update action in GitHub Actions workflow
6 participants