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

Updates "include-publish-npm-package-deployment" pipeline deployment job to be 1ES compliant #23913

Open
wants to merge 60 commits into
base: main
Choose a base branch
from

Conversation

seanimam
Copy link
Contributor

Description

Updates include-publish-npm-package-deployment to be 1ES compliant.

The change moves the code checkout step to a separate job and uploads it as a pipeline artifact because 1ES compliant deployment jobs do not allow checking out a code repository.

Breaking Changes

  • Pipeline has been changed

Reviewer Guidance

  • What variable should I use to signal prod versus non prod in the templateContext section?

@seanimam seanimam requested review from Copilot, tylerbutler and alexvy86 and removed request for Copilot February 24, 2025 17:37
@github-actions github-actions bot added area: build Build related issues base: main PRs targeted against main branch labels Feb 24, 2025
@seanimam seanimam changed the title Updates include-publish-npm-package-deployment to be 1ES compliant Updates include-publish-npm-package-deployment pipeline deployment job to be 1ES compliant Feb 24, 2025
@seanimam seanimam changed the title Updates include-publish-npm-package-deployment pipeline deployment job to be 1ES compliant Updates "include-publish-npm-package-deployment" pipeline deployment job to be 1ES compliant Feb 24, 2025
Copy link
Member

@tylerbutler tylerbutler left a comment

Choose a reason for hiding this comment

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

Apologies if I missed something during earlier discussion. How will the pipelines add git tags to the repo after releases in this new pattern?

steps:
# Checkout the repository
- checkout: self
persistCredentials: true # Necessary for creation of git tags to work
Copy link
Member

Choose a reason for hiding this comment

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

Does this job actually tag the repo?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think tagging remains in the job that does it today (the deployment job that "becomes" a 1ES release job), this extra one wouldn't need this setting. But in https://github.com/microsoft/FluidFramework/pull/23913/files#r1968504949 I argue the new job probably isn't the right solution.

Comment on lines 46 to 65
# This job is necessary to be compliant with 1ES.
# 1ES does not allow checking out a code repo within a deployment job so instead we use this pattern
# of checking out the repo in separate job and uploading it as a pipeline artifact.
- job: checkoutAndUploadThisRepoAsArtifact
steps:
# Checkout the repository
- checkout: self
persistCredentials: true # Necessary for creation of git tags to work
# Copy required files to the artifact staging directory
- task: CopyFiles@2
inputs:
SourceFolder: $(System.DefaultWorkingDirectory)
Contents: '**/*'
TargetFolder: $(Build.ArtifactStagingDirectory)
# Publish the artifact containing the required files from the repository
- task: 1ES.PublishPipelineArtifact@1
inputs:
targetPath: $(Build.ArtifactStagingDirectory)
artifactName: fluidFrameworkRepo

Copy link
Contributor

@alexvy86 alexvy86 Feb 24, 2025

Choose a reason for hiding this comment

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

Hmmm I think this would technically work, but still goes against the spirit of what we're trying to accomplish by complying with the 1ES suggestion, because we would still need to build the build-tools release group in the release job. That is ultimately what we're trying to not do: have to build any code in that job.

So in my mind, what we need to do is:

  • From the build stage, publish the build output of the build-tools release group as an artifact.
    • I don't remember if the current build stage also builds build-tools every time; I think it does it if the pipeline is run with the (default) setting to install/use build-tools from the repo. But we might need to ensure that always happens so we have the artifact available when publishing.
  • In the release job, download that artifact and make the necessary updates to use it.
    • I think instead of using include-install-build-tools.yml, which does all the building and linking of build-tools, we just grab a couple of pieces from that template, adjust them if needed, and use them directly (might just need the npm link line).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: build Build related issues base: main PRs targeted against main branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants