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

Bill of Materials #92

Merged
merged 4 commits into from
Jun 15, 2018
Merged

Bill of Materials #92

merged 4 commits into from
Jun 15, 2018

Conversation

carlossg
Copy link
Contributor

@carlossg carlossg commented Apr 20, 2018

Copy link
Member

@jglick jglick left a comment

Choose a reason for hiding this comment

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

Not really following how you propose this be used concretely in existing Maven infrastructure. Is there a reference implementation that shows tools adapted to use the proposed format?

* Essentials needs to generate a diff for upgrades and download the bits
* Existing ATH uses a war binary that can be built from BoM

Packages should be trackable back to a commit id, but during development the BoM should point to branches or tags, so we would have a BoM as input and a effective BoM as output with all the git refs resolved to specific commit ids.
Copy link
Member

Choose a reason for hiding this comment

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

I am fine with the BoM pointing to a branch during local development—iterative testing on my own computer. Actually more useful is for it to refer to a SNAPSHOT version in that case. But a BoM with a branch name should never be committed, much less pushed—the developer should resolve the reference to a specific commit (if not formal release version) before sharing the contents with other developers, or CI.

Tags are hardly useful I think. If you want to refer to a release version, cut a release. If not, point to a commit hash.

Copy link
Member

Choose a reason for hiding this comment

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

more useful is for it to refer to a SNAPSHOT version in that case

Most useful would be for it to refer to a local file path, triggering a build from sources. I think the WAR packager has an option like this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the goal is to free developers from tedious tasks

a) In my plugin I want to point to core master and automatically keep retesting my plugin when core is updated, I want to know if something in core breaks it, without having to update versions continuously
b) If my plugin PR depends on a PR in core I want to point to that PR and automatically keep retesting my plugin PR when the core PR is updated

Copy link
Member

Choose a reason for hiding this comment

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

I do like the idea of freeing developers from tedious tasks, but as much as I like that idea, I am not sure we here have a clear audit trail/traceability of what versions exactly go to production. And I do deeply dislike the idea of "version range" or something similar. And saying one uses a given branch is similar to this: you don't really know what you're using, and it can change somewhere.

I prefer the GitOps approach described in JEP-305 where things are explicitly updated using a PR for instance. I think we must design things for end-user consumption, then we can think about how to create tooling to make things easier to bump/update locally for quick development turnaround.

Maybe here what you have in mind is not contradictory to this, but I'm just voicing my concern in case.

Developer needs some changes to core or Jenkins internal library or component for downstream use.

* Create PR against core/library.
** PR builder in core/library will publish the artifacts using the git commit id as part of the identifier, for downstream consumption.
Copy link
Member

Choose a reason for hiding this comment

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

This is being handled in JEP-305, so link to that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

** PR builder in core/library will publish the artifacts using the git commit id as part of the identifier, for downstream consumption.
** Would also publish the result BoM with all git references converted to ids.
* Create PR against a plugin
** This PR would include a modified BoM pointing to the PR branch of core/library.
Copy link
Member

Choose a reason for hiding this comment

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

Does not make sense. You would already need to be modifying the pom.xml of the plugin to specify the version of core you wished to consume. (Libraries are handled a bit differently: they need to be consumed in core first, and then that in turned consumed by the plugin.)

So what exactly would the BoM be doing here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The BoM is the source of truth and the tooling would need to update the pom to use the right artifact version (the commit id)

* Create PR against a plugin
** This PR would include a modified BoM pointing to the PR branch of core/library.
* PR builder for plugin/library would process the BoM, creating as output:
** A Jenkins package (WAR and/or Docker image) with the dependencies stated in the BoM.
Copy link
Member

Choose a reason for hiding this comment

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

But this is already handled by Maven without a BoM, where JEP-305 merely makes it easier to create release versions. (The WAR part at least. The Docker image is not created by Maven, but it can take Maven artifacts as inputs.)

** The realized BoM, with all git references converted to ids.
** Dependencies would not need to be rebuilt as they were already published.
* Tests run against this output
* Output artifact is published to repository, for downstream consumption in a way that can be fetched by git commmit id.
Copy link
Member

Choose a reason for hiding this comment

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

Already under implementation in JEP-305.

The current approach to make changes in core, libraries and plugins is too cumbersome, far from Continuous Integration and complicated for contributors, due to the usage of multiple repositories.

Changes typically span more than one repository, causing contributors to manually combine different PRs together.
The goal of this proposal is to move towards a master based delivery process, ensure that core changes don't break plugins and that core changes needed by plugins can be quickly and safely adopted.
Copy link
Member

Choose a reason for hiding this comment

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

Ensuring that core changes do not break plugins (or, more generally, that changes to upstream components do not break downstream components) is already possible with existing tools like PCT. Setting up a CI job to run such tests should not be particularly challenging; the hard part is filtering through what are expected to be hundreds of test failures and getting the test suite to a stable point so that regressions can be triaged.

For version naming there are other options:

* Use Maven SNAPSHOTS
** Automatically deploy snapshots using commit ids (ie. jenkins-core:aabbcc-SNAPSHOT)
Copy link
Member

Choose a reason for hiding this comment

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

As described in JEP-305, Maven snapshots are very problematic and should not be used except as temporary local artifacts.

* Use Maven SNAPSHOTS
** Automatically deploy snapshots using commit ids (ie. jenkins-core:aabbcc-SNAPSHOT)
** Ensure the commit ids are included in the packaging and visible during builds
* Use git modules to point to the master and PR commits
Copy link
Member

Choose a reason for hiding this comment

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

Also discussed in JEP-305.


== Backwards Compatibility

This proposal aims to add new functionality and reuse existing tooling by generating Maven poms and other formats in use today.
Copy link
Member

Choose a reason for hiding this comment

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

Which Maven POMs would be generated and how?


== Infrastructure Requirements

There are no new infrastructure requirements related to this proposal.
Copy link
Member

Choose a reason for hiding this comment

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

Presumably there would be CI requirements.

Copy link
Member

Choose a reason for hiding this comment

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

IMHO we want to create a step in pipeline-library, e.g. via integration with Custom WAR Packager. After that BOM will be accessible to all users within Jenkins infra if needed

Copy link
Member

@rtyler rtyler left a comment

Choose a reason for hiding this comment

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

From a JEP document standpoint this looks mostly well-formed. I'm not sure the Reasoning section has sufficient detail to help myself and others recognize why the YAML format is appropriate.

I think gratuitous links with JEP-305 and cross-references might help, since they're obviously related.

# version: 1.0
components: ...
# other sections can be added and ignored by default
# the realized BoM after refs are resolved
Copy link
Member

Choose a reason for hiding this comment

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

@carlossg I'm having trouble understanding this part of the format. Is this expected to all be within the same yaml file? Or would the "realized BoM" be a separate generated file output?

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 suggest in the same file

Copy link
Member

Choose a reason for hiding this comment

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

Related to my comment in JENKINS-50953, I believe the format for "realized" should contain referenceable URLs, This will make this file digestable and immediately usable by the Jenkins Essentials backend tooling.

# version OR version + ref (version just to keep Maven happy about version order)
ref: master
version: 1.0
components:
Copy link
Member

Choose a reason for hiding this comment

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

While core and plugins are fairly easily understood, I'm not sure what components conceptually maps too. Perhaps it would be helpful to explain these root keys and their purpose below the sample file.

Copy link
Member

Choose a reason for hiding this comment

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

Is a component like a community pipeline shared library? Basically, any code which doesn't fit into the norms of typical Jenkins contributions.

Copy link
Member

Choose a reason for hiding this comment

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

In my cases "components" could be JARs and System Groovy Scripts to be injected. But I agree, it needs to be documented

Copy link
Member

@oleg-nenashev oleg-nenashev left a comment

Choose a reason for hiding this comment

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

"Prototype implementation" section is missing. Without that JEP cannot be accepted as Draft IMHO.

Regarding the technical side, I nominate myself to do some prototype implementation in Custom WAR Packager


==== Changes to Core

Developer needs some changes to core or Jenkins internal library or component for downstream use.
Copy link
Member

Choose a reason for hiding this comment

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

What exactly do you want to support in BOM? Some components like Extras Executable WAR have non-trivial packaging logic, which cannot be easily integrated into Core build. Without https://issues.jenkins-ci.org/browse/JENKINS-48578 it is not possible to patch such component easily, and likely the rebuild is required anyway

* https://github.com/jenkinsci/blueocean-acceptance-test[Blue Ocean acceptance test]
* https://github.com/jenkinsci/plugin-compat-tester[Jenkins plugin compat tester]
* https://github.com/jenkins-infra/evergreen[Essentials evergreen draft]
* https://github.com/jenkinsci/jenkins-war-packager[Jenkins WAR packager] recently started
Copy link
Member

Choose a reason for hiding this comment

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

=== What exists today

* https://github.com/jenkinsci/acceptance-test-harness[Jenkins acceptance test harness]
* https://github.com/jenkinsci/blueocean-acceptance-test[Blue Ocean acceptance test]
Copy link
Member

Choose a reason for hiding this comment

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

Not related to packaging.
For packaging I would rather reference https://github.com/jenkinsci/docker

* https://github.com/jenkinsci/blueocean-acceptance-test[Blue Ocean acceptance test]
* https://github.com/jenkinsci/plugin-compat-tester[Jenkins plugin compat tester]
* https://github.com/jenkins-infra/evergreen[Essentials evergreen draft]
* https://github.com/jenkinsci/jenkins-war-packager[Jenkins WAR packager]
Copy link
Member

Choose a reason for hiding this comment

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

Wrong link again

//
// Uncomment if discussion will occur in forum other than jenkinsci-dev@ mailing list.
//| Discussions-To
//| :bulb: Link to where discussion and final status announcement will occur :bulb:
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Uncomment if discussion will occur in forum other than jenkinsci-dev@ mailing list.


There are no testing issues related to this proposal.

== References
Copy link
Member

Choose a reason for hiding this comment

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

https://github.com/jenkinsci/jep/tree/master/jep/1#required-sections . "Prototype Implementation" section is missing. Without that JEP cannot be accepted as draft. My proposal would be to create a prototype implementation in https://github.com/jenkinsci/custom-war-packager.

@jglick
Copy link
Member

jglick commented Apr 24, 2018

I think we have a fundamental clash of vision for the development workflow. I will try to better explain my perspective.

the goal is to free developers from tedious tasks

That is a reasonable constraint, but it does not have to be at the expense of deterministic, reproducible builds.

My acid test would be this: if /job/my-plugin/job/master/lastBuild/ is blue, and I

echo >> README.md

and file a PR from that, I expect /job/my-plugin/job/PR-123/1/ to also be blue, since I could not possibly have broken anything by doing this. (Unless my build process somehow process data out of the README or something like that, of course.)

If the trivial PR is filed by the plugin maintainer, maybe we could tolerate a failure unrelated to the actual contents of the PR, if you can give up the benefits of git bisect and so on. But if it was filed by an outside contributor, this is really bad. They are being given negative feedback about something which was not their fault, and which they probably do not even comprehend.

In my plugin I want to point to core master and automatically keep retesting my plugin when core is updated, I want to know if something in core breaks it, without having to update versions continuously

Pointing to a nondeterministic branch of core in a BOM file in a plugin is neither necessary for this goal, nor sufficient.

First, it is not sufficient since you would only notice a problem when a new build happens to run on one of the branch projects, either due to a change in master or some new or updated PR. But if the plugin is mostly dormant, months could elapse between such changes, during which breaking core changes could have been “promoted” to weeklies or even LTS releases. It is also not sufficient because the specific tests requested in this plugin’s BOM are only those which you expect are most likely to be broken, based on intuition and historical experience—and would be downstream tests, generally unaffected by changes upstream to this repository.

Second, it is not necessary. We must anyway have a CI job running on some schedule (say, nightly) that runs all tests against master of all components, and some team/process for triaging the failures and assigning them to responsible developers for detailed investigation and correction. In the context of Essentials, we must also be running all stable tests of “essential” components when a new deployment is proposed, which would also reveal any integration problems that had not been caught by an earlier test phase.

In either case, after the immediate problem is corrected (fixing a regression, adapting to it, or correcting a faulty test), we should take two follow-up steps:

  1. Bump the upstream version number in the downstream BOM, so that the downstream repository is now testing against a newer baseline. This should of course be done in a PR to verify that everything passes before merging.
    (Cf. JENKINS-47498: ideally, this could be as simple as incrementing some integer which selects an entire Essentials BOM for import of versions. In other words, a downstream BOM should not need to exhaustively enumerate dozens of individual component versions—just an overall baseline version, plus some overrides corresponding to upstream PRs being consumed.)
  2. Encourage developers to improve upon the late discovery of the problem by reproducing the bug in a more unit-like test in a more upstream repository, to pay down technical debt.

If my plugin PR depends on a PR in core I want to point to that PR and automatically keep retesting my plugin PR when the core PR is updated

This is better done with a script which allows you to both consume the updated upstream PR and make any matching downstream adaptations in a single atomic Git commit: JENKINS-50699.

@bitwiseman
Copy link
Contributor

@carlossg Where's waiting on changes to this document before we can merge. Please update based on feedback and add a comment when you would like to resubmit for approval as Draft.

Copy link
Member

@batmat batmat left a comment

Choose a reason for hiding this comment

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

Some comments.

About the design here: do we have a prototype design started somewhere? It looks like JEP-1 implies that a prototype should be ongoing for draft submission
https://github.com/jenkinsci/jep/tree/master/jep/1#submission

=== Bill of Materials (BoM)

The BoM defines what commits of core, bundled libraries (stapler, remoting) and plugins are delivered and tested together.
They key part is the usage of commit ids and not releases.
Copy link
Member

Choose a reason for hiding this comment

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

they => the


=== What are we missing

* Builds/Tests from master, needed for Essentials
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this be moved below somehow under the Testing section, where it currently says that there's nothing in here about Testing?

* Essentials needs to generate a diff for upgrades and download the bits
* Existing ATH uses a war binary that can be built from BoM

Packages should be trackable back to a commit id, but during development the BoM should point to branches or tags, so we would have a BoM as input and a effective BoM as output with all the git refs resolved to specific commit ids.
Copy link
Member

Choose a reason for hiding this comment

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

I do like the idea of freeing developers from tedious tasks, but as much as I like that idea, I am not sure we here have a clear audit trail/traceability of what versions exactly go to production. And I do deeply dislike the idea of "version range" or something similar. And saying one uses a given branch is similar to this: you don't really know what you're using, and it can change somewhere.

I prefer the GitOps approach described in JEP-305 where things are explicitly updated using a PR for instance. I think we must design things for end-user consumption, then we can think about how to create tooling to make things easier to bump/update locally for quick development turnaround.

Maybe here what you have in mind is not contradictory to this, but I'm just voicing my concern in case.

@carlossg
Copy link
Contributor Author

carlossg commented Apr 27, 2018 via email

@oleg-nenashev
Copy link
Member

I have created JENKINS-51077 for BOM support in Custom WAR packager and started working on the reference implementation

plugins:
- groupId: org.acme
artifactId: acme-plugin
ref: master
Copy link
Member

Choose a reason for hiding this comment

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

Git's remote is not defined here, we cannot easily determine it for plugins NOT hosted in update center

Copy link
Member

@jglick jglick left a comment

Choose a reason for hiding this comment

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

Some more discussion.

components: ...
# other sections can be added and ignored by default
# the realized BoM after refs are resolved
status:
Copy link
Member

Choose a reason for hiding this comment

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

What is status?

- groupId: org.acme
artifactId: acme-plugin
ref: master
# version: 1.0
Copy link
Member

Choose a reason for hiding this comment

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

After some work on JENKINS-50953, I have a suggestion here.

First, ditch the concept of “realized BOM”. This is an aspect of my broader comment made earlier about deterministic builds. So for sake of argument, let us say there is just one BOM format, and it pins particular versions of every component.

Let us also say that the version is simply a Maven version string, as opposed to the URLs suggested by @rtyler which seems overdetermined:

  • makes it harder to apply caches and proxies
  • for any given product build we already have a well-known list of repositories to look in
  • the update tool needs to know that same list of repositories anyway
  • more verbose and error-prone

Then you could have several variants. For a plain old trunk release from maven-release-plugin (“MRP”):

- groupId: io.jenkins.plugins
  artifactId: stuff
  version: 1.23

There is an implied branch: master attribute here which is elided for brevity. For an incremental release from trunk:

- groupId: io.jenkins.plugins
  artifactId: stuff
  version: 1.24-rc9999.abc123def456

For an incremental release from an unmerged PR:

- groupId: io.jenkins.plugins
  artifactId: stuff
  version: 1.24-rc9999.abc123def456
  branch: jglick:refactoring-JENKINS-56789

(Note the syntax matching GitHub conventions.) Even other options are possible, like MRP from a backport branch (in the origin repo):

- groupId: io.jenkins.plugins
  artifactId: stuff
  version: 1.23.1
  branch: stable

Wait, you exclaim! If we are pinning a version, why do we need a branch too? Two reasons.

First, the version update tool needs to know which branch to inspect for candidate versions. Remember that while incremental version numbers are meaningfully comparable within a branch, that is not true between branches, since commit history can be a DAG, so it cannot simply pick the deployed release with the highest $(git rev-list --first-parent --count HEAD); it has to filter them by branch. If we want to be able to run the tool in a mode where it offers updates to every component in the file without further ado, it needs to know if there are some which are currently being taken from something other than master.

Related to that, we ought to be performing automated validation if this BOM is being used for production, such as the Essentials manifest: a CI build of the BOM repo could use a subset of the code in JENKINS-50953 to check that a requested incremental version indeed maps to something already pushed to the designated branch. As alluded to in JEP-305, this is important for security. Suppose you receive a pull request that says

-  version: 2.13-rc735.f7ae668c009b
+  version: 2.13-rc737.789e66b233dd

Sure, that looks OK, right? A plugin update, go for it. But how do you know what this 789e66b233dd commit actually was? Something you trust? Are you going to construct the GitHub commit URL for it and take a look? Probably not, you will just merge and forget. But that might have been malware built from some fly-by-night PR which was closed the moment its artifacts got deployed to Incrementals! By contrast, with the branch system that PR would be marked as a build failure, since the commit is not an ancestor of the current master, and if the pull request looks like

-  version: 2.13-rc735.f7ae668c009b
+  version: 2.13-rc737.789e66b233dd
+  branch: l33thaxor:muahaha

you are going to think twice before merging it.

(A particular BOM like the Essentials manifest may of course wish to use only master commits as a matter of policy, and it might also be receiving routine updates from a bot rather than PRs filed by actual people, but it would be best if there is a single format that can be used safely in different contexts.)

@jglick
Copy link
Member

jglick commented May 7, 2018

Pulling a conversation out of a line item since I think it is a pretty basic question.

The BoM is the source of truth and the tooling would need to update the pom to use the right artifact version (the commit id)

There is no concrete proposal here for how that would happen, or discussion of how that would affect existing developer workflows.

My tentative conclusion from what seems to be the first aspects of a reference implementation in jenkinsci/artifact-manager-s3-plugin#20 (comment) is that we do not need a general BOM format at all. The custom WAR packager needs a small amount of configuration, and Evergreen’s config file is needed to define what goes into production, but I do not see any need for other tools to interact with either of these. It would be useful for the evergreen repo to generate and deploy a Maven BOM (a pom.xml with only <dependencyManagement>) that we could import from other POMs to set defaults for artifact versions: JENKINS-47498.

@oleg-nenashev
Copy link
Member

oleg-nenashev commented May 7, 2018

In jenkinsci/artifact-manager-s3-plugin#20 we would likely need to generate BOM from a BOM Template and pom.xml to make it fancy from user PoV. Integration tests for Essentials components unlikely need BOM, because Custom WAR Packager and Pipeline methods do the job (and it's generally more flexible). But BOM may be still useful as unified format.

From Testing perspective, I am using BOM reference implementation in some internal flow prototypes (actually, only the status section), but I could replace it if needed. CWP lacks support of environments, but it can be added if needed.

@jglick
Copy link
Member

jglick commented May 7, 2018

One thing I can see a BOM adding is a source branch override (replacing master, as in #92 (comment)) for those dependencies taken from unmerged PRs: the 4.0.0 POM schema does not offer any good place to add custom metadata. On the other hand you could just override this mojo configuration in your PR’s patch to pom.xml; for Evergreen security validation we can just assume master.

@bitwiseman
Copy link
Contributor

This JEP is still in a pre-draft state?

@oleg-nenashev said this couldn't be approved as draft until he implemented a prototype. That has been done. I've cleared his request for changes.

@carlossg @oleg-nenashev @jglick @jenkinsci/jep-editors
Is there any reason why this shouldn't be set to draft?

Copy link
Contributor

@bitwiseman bitwiseman left a comment

Choose a reason for hiding this comment

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

Approving as Draft JEP-309.

@bitwiseman bitwiseman merged commit e7f204c into jenkinsci:master Jun 15, 2018
@oleg-nenashev
Copy link
Member

After the prototype we started the discussion whether it is needed, and so far there is no response from the JEP Sponsor. IMHO JEP approval as draft was premature

@oleg-nenashev
Copy link
Member

It's not a big deal, it's always possible to return it to draft or reject later

@bitwiseman
Copy link
Contributor

@oleg-nenashev
While it is fine for JEPs to remain in draft state for an extended period, it is not great for them to remain in "submitted" state for very long. As you say, the JEP can be reject later if needed.

@jglick jglick mentioned this pull request Nov 16, 2022
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.

7 participants