Release notes mappings libraries and in krel release-notes

[puerco]

What type of PR is this?

/kind feature

What this PR does / why we need it:

This is the first effort to add the capability to process mapping files to the release notes package.

The release notes mapping files (format is detailed below) will allow expanding the information in release notes in two ways:

• Modifying original data: The map can supersede release note data defined in the original pull request by overriding certain fields in a file which will eventually end up in the release notes document.

• Adding new fields of information: A map file can define new data structures which can be associated with the release note to add more information to the release note. Current plans include adding security information and a quality score.

Mapping files can come from a variety of sources. This PR adds a new interface MapProvider whose job is to obtain and parse map files. MapProviders can be written to collect release notes files from a variety of sources: reading from a directory (a proof of concept is included), a bucket, a private GitHub repo, an encrypted archive, etc.

The krel release-notes subcommand has been modified with a new --maps-from flag to enable MapProviders when generating release notes. For example, to invoke it with a DirectoryMapProvider we would use the following flags

krel release-notes --create-website-pr --tag v1.19.beta.2 \
   --maps-from=/notes-maps/
# With an hypothetical provider for a bucket
krel release-notes --create-website-pr --tag v1.19.beta.2 \
   --maps-from=gs://k8s-release-notes-bucket

When MapProviders are active, the notes gatherer will query the provider for mappings for a specific note when generating it. Using a new method ApplyMap() the gatherer will modify the release note with data from the map.

The proposed map file format has two parts: one that overrides the original release note data and one that adds data fields, which are free-form data structures keyed in associative array fashion by a keyword. An example:

---
pr:  123
commit: 1a89038915fe77d73bf7c9cfa8f2ce123a464c82
release-note:  
  text: Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. 
  author: kubernetes-ci-robot
  areas:
    - testarea
  kinds:
    - documentation
  sigs:
    - api-machinery
  feature: true
  action_required: false
  release_version: v1.18.4
  documentation:
    - description: Test documentation
      url: https://wkdkjdjkdkfj
      type: kep
datafields:
  # Score is a simple float
  score: 5.2
  # CVE data is a full blown struct
  cve:
    id: CVE-2019-1010260
    description: Really bad SSRF in KCM
    published: 2020-10-05
    issues:
    - 9876
    - 9875

Which issue(s) this PR fixes:

Related to

• Adding security details to release notes #1354
• Release Notes Machine Learning Classifier enhancements#1833

Special notes for your reviewer:

This PR is a proof of concept, should run but is still untested.

I've provided a few sample maps in pkg/notes/maps/testdata/ to illustrate the format and its capabilities.

A simple implementation of a MapProvider is included in this PR: DirectoryMapProvider. This MapProvider is the simplest provider possible: it reads a directory, looking for yaml files and tries to open them as release notes maps.

Does this PR introduce a user-facing change?

* New capability in release notes pkg to read map files which can modify release notes data and add new, arbitrary fields.
* Function `ApplyMap()` was added to `ReleasNote`  type to enable it to get a map and modify itself
* New interface `MapProvider` which gets release notes map files
* New flag in `krel release-notes` to enable map providers: `--maps-from` 

[puerco]

/assign @saschagrunert
/hold

[saschagrunert]

Good good, I added some notes on the general design that we can agree on an overall direction. 👍

[saschagrunert]

The score should be part of the cve

[puerco]

You're right. I thought this was the ML quality score.

The idea was to illustrate a different field. In this case the ML quality score. While cve data would be a whole struct this is only a float .

[saschagrunert]

Okay, I guess we do not have to add anything related to ML for now. In any case I guess the CVE needs a score. The idea was to highlight high impact CVE's or sort them by severity.

(I was thinking about being able to link multiple PRs together, so we could add something like linkedPRs. The overall implementation might be a bit complicated, but features made of multiple PRs are a common use-case in Kubernetes.)

[puerco]

I've added more (hypothetical) examples to the fullmap.yaml file and updated the cve field. Although right now i'm only adjusting how the data would fit in the map. Once we settle how we inject the new fields we can decide on the actual formats for the fields and how they would render.

[puerco]

Thanks for your review @saschagrunert this is precisely what I was hoping for, laying out a bit of code to kick start the design discussion 😀

[puerco]

Hey sascha, I've pushed a new prototype addressing the following:

1. Flags have been updated as we discussed:
   --maps-from=/path/ will create a provider to read a directory to get yaml files. I added a demo hook for a CloudStorageProvider to demonstrate how/where we would add new providers in the future.
2. The parser now support multiple YAML documents from a single file
3. I've updated the sample file fullmap.yaml to better illustrate how I think future data can be aggregated. (All of these are just examples, not proposals for actual uses)
4. The data structure has not been changed yet, except for making fields pointers, Checkout the comment/question on the code review.
5. I've modified the data that the parser returns to support returning multiple maps for the same PR

Needless to say, it is still a rough demo until we settle on a design.

[saschagrunert]

Pretty good, I like the implementation and just have a few minor nits left. Do you wanna merge that PR before the demo or do we wanna demo it first and then gather further feedback?

[puerco]

I've pushed another commit addressing these nits. I think we should merge it because it does not alter the current tools but we need these if we want to produce some output (in a document, not just yamls) we can show.

What do you say if I give it a final polish tonight, squash the commits and resubmit. Then, based on this, we can start thinking how to assert the datafield interfaces and where/how we would add the data (like the CVE info) on the final documents.

Sounds good?

[saschagrunert]

I've pushed another commit addressing these nits. I think we should merge it because it does not alter the current tools but we need these if we want to produce some output (in a document, not just yamls) we can show.

What do you say if I give it a final polish tonight, squash the commits and resubmit. Then, based on this, we can start thinking how to assert the datafield interfaces and where/how we would add the data (like the CVE info) on the final documents.

Sounds good?

Yep, sounds wonderful to me :)

[saschagrunert]

/lgtm

[saschagrunert]

/approve

[puerco]

/kind feature

[hasheddan]

/lgtm

[puerco]

@justaugustus do you think we can merge this one to get the next ones rolling ? :)

[puerco]

OK, this this PR and its following changes are officially in rebase hell.

I was trying to push the enhancement in steps: first the libraries, then the tools and finally the template and its tests. I think that now I will rebase and push these changes again but along the rest of the steps (except the template) in the same PR.

[saschagrunert]

OK, this this PR and its following changes are officially in rebase hell.

I was trying to push the enhancement in steps: first the libraries, then the tools and finally the template and its tests. I think that now I will rebase and push these changes again but along the rest of the steps (except the template) in the same PR.

Whoops big sorry for that! Let's try to get those changes in ASAP.

[puerco]

No worries :) We cannot stop pushing ahead just for a small part getting stuck. Lets code some more! 💪

[saschagrunert]

/lgtm

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hasheddan, puerco, saschagrunert

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:

• OWNERS [saschagrunert]

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

[saschagrunert]

@puerco can we lift the hold?

[puerco]

Sorry, yes. It was intended to be there while we decided on the approach. This one is fairly innocuous as it does not alter the output until we change the template.
/hold cancel