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

Fixes an issue where specifying repo for Big Bang extension fails #1613

Merged
merged 2 commits into from
Apr 18, 2023

Conversation

dgershman
Copy link
Contributor

@dgershman dgershman commented Apr 18, 2023

Description

This fixes an issue where when specifying a repo in the Big Bang extension (during zarf package create, causes a packaging related error.

ERROR:  Failed to create package: unable to process extensions: unable to process bigbang extension: unable to build kustomization: git cmd = '/usr/local/bin/git fetch --depth=1 origin 2.0.0-beta2': exit status 128

Here is a yaml snippet example for zarf.yaml that reproduces this error.

kind: ZarfPackageConfig
metadata:
  name: big-bang-example
  description: "Deploy Big Bang Core"
  version: 2.0.0-beta2
  url: https://p1.dso.mil/products/big-bang
  # Big Bang / Iron Bank are only amd64
  architecture: amd64

variables:
  - name: DOMAIN
    default: "bigbang.dev"
    prompt: false

components:
  - name: bigbang
    required: true
    extensions:
      bigbang:
        repo: https://github.com/radiusmethod/bigbang.git
        version: 2.0.0-beta2
        valuesFiles:
          # Istio configs
...

Related Issue

N/A

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Other (security config, docs update, etc)

Checklist before merging

@netlify
Copy link

netlify bot commented Apr 18, 2023

Deploy Preview for zarf-docs ready!

Name Link
🔨 Latest commit 27a59d9
🔍 Latest deploy log https://app.netlify.com/sites/zarf-docs/deploys/643df9034ba55e00088b26d7
😎 Deploy Preview https://deploy-preview-1613--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 settings.

@YrrepNoj
Copy link
Contributor

YrrepNoj commented Apr 18, 2023

I haven't tried to replicate this issue locally yet but I feel like we (I?) might be missing something here. The only time the getFlux( ... ) function that this PR is updating is getting called is in bigbang.go after we do the exact same check for an empty cfg.Repo

https://github.com/defenseunicorns/zarf/blob/3ec78a822a8e90b4de81c23fae06b7d0e99a3346/src/extensions/bigbang/bigbang.go#L61

I can spend more time to try to replicate this tomorrow but wanted to leave this comment incase someone saw something I was missing.

Note: I think checking for an empty cfg.Repo here is still a good idea, I just want to make sure I fully understand what the underlying issue is.

@dgershman
Copy link
Contributor Author

dgershman commented Apr 18, 2023

The issue is that when getFlux() is called, the inner function call is referencing the constant bbRepo again, which means the override is being ignored.

https://github.com/defenseunicorns/zarf/blob/main/src/extensions/bigbang/flux.go#L45

@YrrepNoj
Copy link
Contributor

Ahh thanks, my eyes completely glossed over that

Copy link
Contributor

@Racer159 Racer159 left a comment

Choose a reason for hiding this comment

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

lgtm as well

@Racer159 Racer159 merged commit 85ee015 into zarf-dev:main Apr 18, 2023
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.

3 participants