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

feat!: remove component webhooks #3066

Merged
merged 5 commits into from
Oct 7, 2024
Merged

Conversation

AustinAbro321
Copy link
Contributor

@AustinAbro321 AustinAbro321 commented Oct 3, 2024

Description

This removes component webhooks. An alpha feature that did not see significant adoption

This comes with changes for the the secret generated for each package on deploy. The secret is no longer created / updated at the start and end of the component deploy, now it is only updated at the start. This also changes the deployedComponents object within that secret. Status and observed generation were introduced for component webhooks so they are removed. The deployedComponent json changes from this

"deployedComponents": [
   {
      "name": "flux",
      "installedCharts": [
        {
          "namespace": "flux",
          "chartName": "zarf-766baa529a869b00e592b0dea6712a8f6bd3fd97"
        }
      ],
      "status": "Succeeded",
      "observedGeneration": 1
    },
    {
      "name": "podinfo-via-flux-git",
      "installedCharts": [
        {
          "namespace": "podinfo-git",
          "chartName": "zarf-4a46e12b4a2ff339792184dbd300f33030826914"
        }
      ],
      "status": "Succeeded",
      "observedGeneration": 2
    }
  ]

to this

"deployedComponents": [
    {
      "name": "flux",
      "installedCharts": [
        {
          "namespace": "flux",
          "chartName": "zarf-766baa529a869b00e592b0dea6712a8f6bd3fd97"
        }
      ]
    },
    {
      "name": "podinfo-via-flux-git",
      "installedCharts": [
        {
          "namespace": "podinfo-git",
          "chartName": "zarf-4a46e12b4a2ff339792184dbd300f33030826914"
        }
      ]
    }
  ]

Related Issue

Fixes #2610

Checklist before merging

Signed-off-by: Austin Abro <[email protected]>
Copy link

netlify bot commented Oct 3, 2024

Deploy Preview for zarf-docs ready!

Name Link
🔨 Latest commit d026940
🔍 Latest deploy log https://app.netlify.com/sites/zarf-docs/deploys/66ffe2d4e23a9d000821c719
😎 Deploy Preview https://deploy-preview-3066--zarf-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

codecov bot commented Oct 3, 2024

Codecov Report

Attention: Patch coverage is 0% with 5 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/pkg/packager/deploy.go 0.00% 3 Missing ⚠️
src/pkg/cluster/zarf.go 0.00% 2 Missing ⚠️
Files with missing lines Coverage Δ
src/cmd/common/viper.go 0.00% <ø> (ø)
src/cmd/dev.go 0.00% <ø> (ø)
src/cmd/initialize.go 0.00% <ø> (ø)
src/cmd/package.go 0.00% <ø> (ø)
src/types/k8s.go 66.66% <ø> (ø)
src/pkg/cluster/zarf.go 14.73% <0.00%> (-1.32%) ⬇️
src/pkg/packager/deploy.go 9.23% <0.00%> (+0.20%) ⬆️

@AustinAbro321 AustinAbro321 changed the title remove component webhooks feat!: remove component webhooks Oct 3, 2024
@@ -183,13 +175,6 @@ func (p *Packager) deployComponents(ctx context.Context) ([]types.DeployedCompon
deployedComponents = append(deployedComponents, deployedComponent)
idx := len(deployedComponents) - 1

// Update the package secret to indicate that we are attempting to deploy this component
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now that we no longer have component webhooks there is no reason for us to update the secret before the component is deployed

Signed-off-by: Austin Abro <[email protected]>
@AustinAbro321 AustinAbro321 marked this pull request as ready for review October 4, 2024 13:10
@AustinAbro321 AustinAbro321 requested review from a team as code owners October 4, 2024 13:10
@phillebaba
Copy link
Member

This seems to all look good. My only question is why we are remove the generation counter? Is it specific to the component webhook only?

@AustinAbro321
Copy link
Contributor Author

@phillebaba It's in the package secret so technically others could look at it, but it was added in Zarf so component webhooks could use it. I noticed that it was added in the same PR that introduced component webhooks

Copy link
Member

@phillebaba phillebaba left a comment

Choose a reason for hiding this comment

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

LGTM

@phillebaba phillebaba added this pull request to the merge queue Oct 7, 2024
Merged via the queue into main with commit 66283b4 Oct 7, 2024
40 checks passed
@phillebaba phillebaba deleted the component-webhooks-remove branch October 7, 2024 15:46
mjnagel pushed a commit to mjnagel/zarf that referenced this pull request Oct 21, 2024
Signed-off-by: Austin Abro <[email protected]>
Signed-off-by: Micah Nagel <[email protected]>
ittacco pushed a commit to ittacco/zarf that referenced this pull request Nov 9, 2024
Jneville0815 pushed a commit to radiusmethod/zarf that referenced this pull request Dec 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove component webhooks
2 participants