Ane 926 release group #1399

JeffreyHuynh1 · 2024-03-21T23:53:01Z

Overview

The CLI should offer a way to create release groups from the command line. Creating and deleting other org-level objects should be done through their own command.

Acceptance criteria

There is a way to create and delete a release group from the CLI.
Emit audit logs on release group creation in CORE

Testing plan

example .fossa.yml:

releaseGroup:
  title: example-title
  release: example-release-title
  releaseGroupProjects:
    - projectLocator: custom+1/git@github.com/example
      projectRevision:   12345 
      projectBranch: main
  licensePolicy: license-policy-name 
  securityPolicy: security-policy-name
  qualityPolicy: quality-policy-name
  teams:
      - team1
      - team2

Create a release group:

fossa release-group create --title example-title --release example-release-title --project-locator custom+1/git@github.com/example --project-revision 12345 --project-branch main
fossa release-group create -c /path/to/.fossa.yml/

Add projects to a release group:

fossa release-group add-projects --title example-title --release example-release-title --project-locator custom+1/git@github.com/example --project-revision 12345 --project-branch main
fossa release-group add-projects -c /path/to/.fossa.yml/

Delete a release group:

fossa release-group delete --title example-title

Delete a release group release:

fossa release-group delete-release --title example-title --release example-release-title

Risks

Metrics

References

ANE-926

Checklist

I added tests for this PR's change (or explained in the PR description why tests don't make sense).
If this PR introduced a user-visible change, I added documentation into docs/.
If this PR added docs, I added links as appropriate to the user manual's ToC in docs/README.ms and gave consideration to how discoverable or not my documentation is.
If this change is externally visible, I updated Changelog.md. If this PR did not mark a release, I added my changes into an # Unreleased section at the top.
If I made changes to .fossa.yml or fossa-deps.{json.yml}, I updated docs/references/files/*.schema.json AND I have updated example files used by fossa init command. You may also need to update these if you have added/removed new dependency type (e.g. pip) or analysis target type (e.g. poetry).
If I made changes to a subcommand's options, I updated docs/references/subcommands/<subcommand>.md.

JeffreyHuynh1 · 2024-03-21T23:57:58Z

src/App/Fossa/Config/Common.hs

+deprecateReleaseGroupMetadata :: Has Diagnostics sig m => ProjectMetadata -> m ProjectMetadata
+deprecateReleaseGroupMetadata projectMetadata = do
+  case (projectReleaseGroup projectMetadata) of
+    Nothing -> pure projectMetadata
+    Just _ -> do
+      warn deprecationMessage
+      pure $ removeReleaseGroupMetadata projectMetadata
+  where
+    removeReleaseGroupMetadata :: ProjectMetadata -> ProjectMetadata
+    removeReleaseGroupMetadata project = project{projectReleaseGroup = Nothing}
+
+    deprecationMessage :: Text
+    deprecationMessage =
+      renderIt $
+        vsep
+          [annotate (color Red) "Release group options for this command have been deprecated. Refer to `fossa release-group` subcommands to interact with FOSSA release groups."]


Thinking more about this, not sure if we want to deprecate this on analyze. There was a customer that was asking to update the release projects upon scan and if we deprecate this, they wouldn't be able to do this.

Could you characterize this more? The reason not to deprecate these isn't obvious to me. It seems like they could just analyze and update the release projects in a separate command after?

That's true, you would be able to run analyze and then update the release projects with release-group add-projects, but I think the customer was looking to only run fossa analyze to have their release group project updated.

slack thread

It sounds like deprecating them would not totally prevent the customer from updating a project in a release group. I think it's OK in that case to say that they'll need to use these new commands even if they have a preference for analyze.

That sounds good to me

docs/references/subcommands/release-group.md

docs/references/subcommands/release-group/create.md

csasarak · 2024-03-25T21:11:45Z

src/App/Fossa/Config/Common.hs

+deprecateReleaseGroupMetadata :: Has Diagnostics sig m => ProjectMetadata -> m ProjectMetadata
+deprecateReleaseGroupMetadata projectMetadata = do
+  case (projectReleaseGroup projectMetadata) of
+    Nothing -> pure projectMetadata
+    Just _ -> do
+      warn deprecationMessage
+      pure $ removeReleaseGroupMetadata projectMetadata
+  where
+    removeReleaseGroupMetadata :: ProjectMetadata -> ProjectMetadata
+    removeReleaseGroupMetadata project = project{projectReleaseGroup = Nothing}
+
+    deprecationMessage :: Text
+    deprecationMessage =
+      renderIt $
+        vsep
+          [annotate (color Red) "Release group options for this command have been deprecated. Refer to `fossa release-group` subcommands to interact with FOSSA release groups."]


Could you characterize this more? The reason not to deprecate these isn't obvious to me. It seems like they could just analyze and update the release projects in a separate command after?

src/App/Fossa/Config/ReleaseGroup/Create.hs

src/App/Fossa/ReleaseGroup/DeleteRelease.hs

src/App/Types.hs

src/Fossa/API/Types.hs

test/App/Fossa/Config/ReleaseGroup/CreateSpec.hs

csasarak · 2024-03-26T16:22:13Z

docs/references/files/fossa-yml.md

+    - projectLocator: custom+123/git@github.com/fossas/fossa-cli
+      projectRevision: "12345"
+      projectBranch: master 
+    - projectLocator: custom+123/git@github.com/example


We discussed project id vs. locator with respect to modifying projects and I think we should do the same here. This is technically an entirely new config block so there's no compatibility issues but users are already used to using projectId elsewhere.

I think it'd be good to allow/prefer projectId and make projectLocator available for people who really need it.

I think allowing projectId makes sense when modifying projects as that field was already present in the project config block. However, in this case, because we are introducing a new config block I would rather constrain users to using projectLocator and make it simple. I think if we get requests to include projectId as an option, then we can go ahead and add it, but for now I would like to keep it as is. What do you think?

I think it's more confusing that way for people familiar with project ids or who want to make use of both project's and release groups in a .fossa.yml. If I make a project block and then try to copy some it's project id into a release group block it won't work. It isn't immediately obvious why a projectId isn't good enough even though thus far it's worked everywhere else in .fossa.yml. Yet, I'm required to at least be able to discern the difference between the two to use this functionality. The docs in this PR explain what the difference is but not why the difference is important or necessary.

I think the ideal way would be to use projectId as much as possible and provide projectLocator for cases where someone really needs it. I think that the majority of cases would be handled by this and it's easier to understand.

If you disagree, I'm OK with moving forward with it this way and seeing what the response is, but please edit the project ID and project locator docs to describe the different semantics of these values. Maybe mention directly in the release group docs that project id won't work. Additionally, the existing project id docs describe it as a unique id for a project which after talking it over, I think isn't true. Could you edit that description to be specific?

Sounds good I updated the docs!

meghfossa

I see that chris already added comments - here are some of them from my review; Also test-plan's .fossa.yml is incorrect - e.g. projectLocator

src/App/Fossa/ReleaseGroup/Create.hs

docs/references/files/fossa-yml.md

meghfossa · 2024-03-26T16:23:47Z

src/App/Fossa/ReleaseGroup/Create.hs

+  policies <- getPolicies
+  teams <- getTeams
+
+  maybeLicensePolicyId <- retrievePolicyId (releaseGroupLicensePolicy releaseGroupRevision) LICENSING policies


[question] why are we doing policy resolution logic in cmdline opposed to backend?

Core wanted me to use their existing endpoints for my work and they only have endpoints to retrieve policies by id, so I had to do policy resolution here.

src/App/Fossa/ReleaseGroup/Create.hs

src/Control/Carrier/FossaApiClient/Internal/FossaAPIV1.hs

csasarak

This looks pretty good. I can approve after you add some clarification in the docs on the differences in semantics between projectId and projectLocator as well as some description that projectIds are only unique for the purposes of analyze.

csasarak · 2024-03-28T23:52:14Z

docs/references/files/fossa-yml.md

+    - projectLocator: custom+123/git@github.com/fossas/fossa-cli
+      projectRevision: "12345"
+      projectBranch: master 
+    - projectLocator: custom+123/git@github.com/example


I think it's more confusing that way for people familiar with project ids or who want to make use of both project's and release groups in a .fossa.yml. If I make a project block and then try to copy some it's project id into a release group block it won't work. It isn't immediately obvious why a projectId isn't good enough even though thus far it's worked everywhere else in .fossa.yml. Yet, I'm required to at least be able to discern the difference between the two to use this functionality. The docs in this PR explain what the difference is but not why the difference is important or necessary.

I think the ideal way would be to use projectId as much as possible and provide projectLocator for cases where someone really needs it. I think that the majority of cases would be handled by this and it's easier to understand.

If you disagree, I'm OK with moving forward with it this way and seeing what the response is, but please edit the project ID and project locator docs to describe the different semantics of these values. Maybe mention directly in the release group docs that project id won't work. Additionally, the existing project id docs describe it as a unique id for a project which after talking it over, I think isn't true. Could you edit that description to be specific?

csasarak · 2024-03-29T00:03:32Z

src/App/Fossa/Config/Common.hs

+deprecateReleaseGroupMetadata :: Has Diagnostics sig m => ProjectMetadata -> m ProjectMetadata
+deprecateReleaseGroupMetadata projectMetadata = do
+  case (projectReleaseGroup projectMetadata) of
+    Nothing -> pure projectMetadata
+    Just _ -> do
+      warn deprecationMessage
+      pure $ removeReleaseGroupMetadata projectMetadata
+  where
+    removeReleaseGroupMetadata :: ProjectMetadata -> ProjectMetadata
+    removeReleaseGroupMetadata project = project{projectReleaseGroup = Nothing}
+
+    deprecationMessage :: Text
+    deprecationMessage =
+      renderIt $
+        vsep
+          [annotate (color Red) "Release group options for this command have been deprecated. Refer to `fossa release-group` subcommands to interact with FOSSA release groups."]


It sounds like deprecating them would not totally prevent the customer from updating a project in a release group. I think it's OK in that case to say that they'll need to use these new commands even if they have a preference for analyze.

csasarak · 2024-03-29T00:05:04Z

src/App/Fossa/Config/Common.hs

@@ -482,6 +497,14 @@ commonOpts =
          , boldItalicized "Default: " <> coloredBoldItalicized Green "full"
          ]

+configHelp :: Maybe (Doc AnsiStyle)


I don't see a reason for these to be Maybe since it doesn't seem like there's any possibility that they will be Nothing. This forces consumers of that data to handle the case of Nothing regardless though.

These help docs are of type Maybe due to Options.Applicative helpDoc function. It required aMaybe type.

csasarak · 2024-03-29T00:07:04Z

src/App/Fossa/Config/ReleaseGroup/AddProjects.hs

+cliParser =
+  AddProjectsOpts
+    <$> releaseGroupCommonOpts
+    <*> optional (strOption (applyFossaStyle <> long "config" <> short 'c' <> helpDoc configHelp))


I'm fine with a ticket, but I was only asking for sharing for the config values defined in this PR. That we can't get sharing for everything doesn't mean it isn't good to do it where it's possible and relatively easy.

src/App/Fossa/Config/ReleaseGroup/AddProjects.hs

csasarak · 2024-03-29T00:15:12Z

src/App/Fossa/ReleaseGroup/Common.hs

+    [] -> pure Nothing
+    [releaseGroup] -> pure . Just $ releaseGroupId releaseGroup
+    (_ : _ : _) ->
+      errHelp ("Navigate to the FOSSA web UI to rename your release groups so that they are unqiue" :: Text)


Suggested change

errHelp ("Navigate to the FOSSA web UI to rename your release groups so that they are unqiue" :: Text)

errHelp ("Navigate to the FOSSA web UI to rename your release groups so that they are unique." :: Text)

Relatedly, as you find things that you think should be constrained in FOSSA's data-model to be unique, can you make tickets if you haven't yet?

src/App/Fossa/ReleaseGroup/Common.hs

JeffreyHuynh1 · 2024-03-29T20:37:10Z

docs/references/files/fossa-yml.md

+### What is the difference between project ID and project locator?
+
+In the CLI, project ID refers to a specific portion of a project locator. When running `fossa analyze` on a project for the first time, if project ID is not directly configured, it will default to:
+
+- Git: The CLI will look for a `.git/config` file and set the ID to the project's remote "origin" url.
+- SVN: The CLI will run `svn info` and use the "Repository Root".
+- No VCS (Version control system): The ID will be set to the name of the project's directory.
+
+<img src="../../assets/project-id-example.png">
+
+
+After project ID is set, the CLI will use that value to construct a project locator on first time analysis. A project locator is a unique ID that the FOSSA API will use to reference a project across FOSSA.
+
+> NOTE: Projects uploaded through the CLI will have `custom` embedded into the project locator. 
+
+<img src="../../assets/project-locator-example.png">
+
+### When do I use project ID vs project locator?
+
+Project ID should be used when referencing projects uploaded via the CLI. In most cases, providing just a project ID is sufficient. However, when referencing projects uploaded through avenues outside the CLI (Quick Import, Archive Upload, etc), providing just a project ID fails. This is due to the nature of FOSSA's project naming conventions as project locators are constructed differently depending on how a project was uploaded. There is no way to tell at runtime which project a user is referencing when only given a project ID (i.e. Is this a project uploaded via the CLI or Archive Upload?).
+
+In `fossa release-group` project locator is used to reference projects that will be added to your release group's release. By using project locator, all FOSSA project upload flows are covered and the CLI is able to determine the specific project a user is trying to add to a release. 


Let me know if this makes sense or if I should word this differently.

Reviewed by Chris

JeffreyHuynh1 added 18 commits February 29, 2024 14:00

adding create command

c34b0d4

initial implementation

998997f

Merge branch 'master' into ANE-926-Create-Release-Group

22a6f27

release group commands

0c75b8b

merge master

991222b

update

ca8d8ed

fix test

c8520c2

update changelog

29ec86e

update

d1a86bb

address pr comments

8dd6cb8

update fossa-yml

fc2ea6a

update

00522be

remove reference to project id for release-group subcommands

0db9e1d

update

1e0cb87

update

5bdc508

update implementation to use existing release group endpoints from core

64b3dba

update changelog

daa13f5

update

536cfa0

JeffreyHuynh1 requested a review from meghfossa March 21, 2024 23:53

JeffreyHuynh1 marked this pull request as ready for review March 21, 2024 23:53

JeffreyHuynh1 requested a review from a team as a code owner March 21, 2024 23:53

update change log

d66f4ed

JeffreyHuynh1 commented Mar 21, 2024

View reviewed changes

typos

f93b10d

csasarak requested changes Mar 26, 2024

View reviewed changes

meghfossa previously requested changes Mar 26, 2024

View reviewed changes

JeffreyHuynh1 added 4 commits March 27, 2024 12:12

address pr comments

2e50224

Merge branch 'master' into ANE-926-Release-Group

cdbb5c6

update create and add-project docs

f9a9c64

Fix lint and update tests

a7ebba8

JeffreyHuynh1 added 2 commits March 27, 2024 12:31

fix typo

d408c4c

remove redundant import

c7c6e3b

JeffreyHuynh1 requested review from csasarak and meghfossa March 27, 2024 19:33

csasarak requested changes Mar 29, 2024

View reviewed changes

JeffreyHuynh1 added 3 commits March 29, 2024 13:29

address pr comments

b2ba8be

Merge branch 'master' into ANE-926-Release-Group

72ab83b

address pr comments

7ffa5ef

JeffreyHuynh1 requested a review from csasarak March 29, 2024 20:35

JeffreyHuynh1 commented Mar 29, 2024

View reviewed changes

csasarak approved these changes Mar 29, 2024

View reviewed changes

JeffreyHuynh1 merged commit 5815f76 into master Mar 29, 2024
16 checks passed

JeffreyHuynh1 deleted the ANE-926-Release-Group branch March 29, 2024 23:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ane 926 release group #1399

Ane 926 release group #1399

JeffreyHuynh1 commented Mar 21, 2024 •

edited

JeffreyHuynh1 Mar 21, 2024

csasarak Mar 25, 2024

JeffreyHuynh1 Mar 27, 2024

csasarak Mar 29, 2024

JeffreyHuynh1 Mar 29, 2024

csasarak Mar 25, 2024

csasarak Mar 26, 2024

JeffreyHuynh1 Mar 27, 2024

csasarak Mar 28, 2024

JeffreyHuynh1 Mar 29, 2024

meghfossa left a comment

meghfossa Mar 26, 2024

JeffreyHuynh1 Mar 27, 2024

csasarak left a comment

csasarak Mar 28, 2024

csasarak Mar 29, 2024

csasarak Mar 29, 2024

JeffreyHuynh1 Mar 29, 2024

csasarak Mar 29, 2024

csasarak Mar 29, 2024

csasarak Mar 29, 2024

JeffreyHuynh1 Mar 29, 2024

	errHelp ("Navigate to the FOSSA web UI to rename your release groups so that they are unqiue" :: Text)
	errHelp ("Navigate to the FOSSA web UI to rename your release groups so that they are unique." :: Text)

Ane 926 release group #1399

Ane 926 release group #1399

Conversation

JeffreyHuynh1 commented Mar 21, 2024 • edited

Overview

Acceptance criteria

Testing plan

Risks

Metrics

References

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

meghfossa left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

csasarak left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JeffreyHuynh1 commented Mar 21, 2024 •

edited