buildpacks/lifecycle

Buildpack API deprecation warning not shown for transitive dependency buildpacks

Opened this issue · 3 comments

edmorley commented

Summary

If a buildpack is using a deprecated Buildpack API version, lifecycle outputs a warning like so during detect:

===> DETECTING
Warning: Buildpack 'heroku/nodejs-npm@2.3.0' requests deprecated API '0.6'

However, if the buildpack in question isn't in the top level buildpack group, and instead is a transitive dependency of a composite buildpack, then the warning isn't shown.

It seems that the check here isn't traversing the full buildpack order group graph:

lifecycle/cmd/lifecycle/main.go

Lines 136 to 143 in dbd27bd

func verifyBuildpackApis(group buildpack.Group) error {
for _, bp := range group.Group {
if err := cmd.VerifyBuildpackAPI(buildpack.KindBuildpack, bp.String(), bp.API, cmd.DefaultLogger); err != nil { // FIXME: when exporter is extensions-aware, this function call should be modified to provide the right module kind
return err
}
}
return nil
}

lifecycle/cmd/apis.go

Lines 35 to 66 in dbd27bd

// VerifyBuildpackAPI given a Buildpack API version and relevant information for logging
// will error if the requested version is unsupported,
// and will log a warning, error, or do nothing if the requested version is deprecated,
// depending on if the configured deprecation mode is "warn", "error", or "silent", respectively.
func VerifyBuildpackAPI(kind, name, requestedVersion string, logger log.Logger) error {
requested, err := api.NewVersion(requestedVersion)
if err != nil {
return FailErrCode(
nil,
CodeForIncompatibleBuildpackAPI,
fmt.Sprintf("parse buildpack API '%s' for %s '%s'", requested, strings.ToLower(kind), name),
)
}
if api.Buildpack.IsSupported(requested) {
if api.Buildpack.IsDeprecated(requested) {
switch DeprecationMode {
case ModeQuiet:
break
case ModeError:
logger.Errorf("%s '%s' requests deprecated API '%s'", kind, name, requestedVersion)
logger.Errorf("Deprecated APIs are disabled by %s=%s", EnvDeprecationMode, ModeError)
return buildpackAPIError(kind, name, requestedVersion)
case ModeWarn:
logger.Warnf("%s '%s' requests deprecated API '%s'", kind, name, requestedVersion)
default:
logger.Warnf("%s '%s' requests deprecated API '%s'", kind, name, requestedVersion)
}
}
return nil
}
return buildpackAPIError(kind, name, requestedVersion)
}

Reproduction

Steps
  1. mkdir testcase && cd $_
  2. pack build test --builder heroku/builder-classic:22 --buildpack heroku/nodejs-npm@2.3.0
  3. pack build test --builder heroku/builder-classic:22 --buildpack heroku/nodejs-function@2.3.0
Current behavior

At step 2 (where the buildpack is referenced directly), a deprecation warning is correctly shown:

===> DETECTING
Warning: Buildpack 'heroku/nodejs-npm@2.3.0' requests deprecated API '0.6'
Timer: Detector started at 2023-11-15T08:45:46Z
======== Results ========
fail: heroku/nodejs-npm@2.3.0

At step 3 (where the buildpack with the deprecated Buildpack API version is instead a transitive dependency of the composite buildpack heroku/nodejs-function), the deprecation warning is missing:

===> DETECTING
Timer: Detector started at 2023-11-15T08:45:18Z
======== Results ========
pass: heroku/nodejs-engine@2.3.0
fail: heroku/nodejs-npm@2.3.0
fail: heroku/nodejs-function-invoker@2.3.0
Expected behavior

For the deprecation warning about Buildpack API version to be shown regardless of whether the buildpack in question is referenced directly, or is a transitive dependency of a composite buildpack.

Context

lifecycle version

0.17.2 (only since 0.18.x has since dropped support for Buildpack API 0.6, meaning there are no deprecated versions left at the moment with which the issue can be demonstrated).

platform version(s)

Pack 0.32.1+git-b14250b.build-5241

anything else?

The buildpacks in question:
https://github.com/heroku/buildpacks-nodejs/blob/v2.3.0/buildpacks/npm/buildpack.toml
https://github.com/heroku/buildpacks-nodejs/blob/v2.3.0/meta-buildpacks/nodejs-function/buildpack.toml

natalieparellano commented

Thanks for reporting this @edmorley. On a side note, does it make sense for a composite buildpack to declare a Buildpack API version? I guess buildpack.toml has to follow some schema... but still, it feels wrong. Maybe a question for another day...

edmorley commented

Good question. We need some way to version the schema, but using something like schema-version (like project.toml uses) for composite buildpacks would seem a bit strange when component buildpacks don't use schema-version and instead use the Buildpack API version to represent a combination of schema version plus implementation details version.

I suppose even though composite buildpacks don't implement detect/build, the order grouping is still quite Buildpack API spec specific -- if we ever changed how concepts like groups or optional worked, then it would seem like more than just a schema change, so at that point linking that change to an actual Buildpack API version would presumable make sense. As such, perhaps it's fine for composite buildpacks to have an API version?

  • 👍1
edmorley commented

This issue is currently in the lifecycle 0.17.3 milestone, but (a) this issue it should probably be in one of the newer milestones, (b) the lifecycle 0.17.3 milestone should probably be closed out, given that version was already released?

Also, ref this issue: I don't think adding a Buildpack API version to a composite buildpack is the correct fix. I think the deprecation check should always iterate through the dependent buildpacks - since there's would be no guarantee that the composite buildpack even uses the same Buildpack API version as it's component buildpack dependents.

  • 👍1