fix verify stage for helmfile when use oci as chart (#1661)

* fix verify stage for helmfile when use oci as chart

Signed-off-by: yxxhero <aiopsclub@163.com>

pkg/state/state.go

@@ -2579,11 +2579,33 @@ func (st *HelmState) appendExtraSyncFlags(flags []string, opt *SyncOpts) []strin
 	return flags
 }
 
+// appendVerifyFlags append the --verify flags related to verify
+func (st *HelmState) appendVerifyFlags(flags []string, release *ReleaseSpec) []string {
+	repo, _ := st.GetRepositoryAndNameFromChartName(release.Chart)
+	switch {
+	case release.Verify != nil:
+		// If the release has a verify flag, use it
+		v := *release.Verify
+		if v {
+			flags = append(flags, "--verify")
+		}
+		return flags
+	case repo != nil && repo.Verify:
+		flags = append(flags, "--verify")
+	case st.HelmDefaults.Verify:
+		flags = append(flags, "--verify")
+	}
+	return flags
+}
+
 // appendKeyringFlags append all the helm command-line flags related to keyring
 func (st *HelmState) appendKeyringFlags(flags []string, release *ReleaseSpec) []string {
+	repo, _ := st.GetRepositoryAndNameFromChartName(release.Chart)
 	switch {
 	case release.Keyring != "":
 		flags = append(flags, "--keyring", release.Keyring)
+	case repo != nil && repo.Keyring != "":
+		flags = append(flags, "--keyring", repo.Keyring)
 	case st.HelmDefaults.Keyring != "":
 		flags = append(flags, "--keyring", st.HelmDefaults.Keyring)
 	}
@@ -2642,13 +2664,6 @@ func (st *HelmState) timeoutFlags(release *ReleaseSpec) []string {
 
 func (st *HelmState) flagsForUpgrade(helm helmexec.Interface, release *ReleaseSpec, workerIndex int, opt *SyncOpts) ([]string, []string, error) {
 	flags := st.chartVersionFlags(release)
-
-	if release.Verify != nil && *release.Verify || release.Verify == nil && st.HelmDefaults.Verify {
-		flags = append(flags, "--verify")
-	}
-
-	flags = st.appendKeyringFlags(flags, release)
-
 	if release.EnableDNS != nil && *release.EnableDNS || release.EnableDNS == nil && st.HelmDefaults.EnableDNS {
 		flags = append(flags, "--enable-dns")
 	}
@@ -2656,6 +2671,12 @@ func (st *HelmState) flagsForUpgrade(helm helmexec.Interface, release *ReleaseSp
 	flags = st.appendWaitFlags(flags, release, opt)
 	flags = st.appendWaitForJobsFlags(flags, release, opt)
 
+	// non-OCI chart should be verified here
+	if !st.IsOCIChart(release.Chart) {
+		flags = st.appendVerifyFlags(flags, release)
+		flags = st.appendKeyringFlags(flags, release)
+	}
+
 	flags = append(flags, st.timeoutFlags(release)...)
 
 	if release.Force != nil && *release.Force || release.Force == nil && st.HelmDefaults.Force {
@@ -3716,6 +3737,11 @@ func (st *HelmState) getOCIChart(release *ReleaseSpec, tempDir string, helm helm
 		st.logger.Debugf("chart already exists at %s", chartPath)
 	} else {
 		flags := st.chartOCIFlags(release)
+
+		// apprnd flags about keyring and verify
+		flags = st.appendVerifyFlags(flags, release)
+		flags = st.appendKeyringFlags(flags, release)
+
 		err := helm.ChartPull(qualifiedChartName, chartPath, flags...)
 		if err != nil {
 			return nil, err
@@ -3737,6 +3763,19 @@ func (st *HelmState) getOCIChart(release *ReleaseSpec, tempDir string, helm helm
 	return &chartPath, nil
 }
 
+// IsOCIChart returns true if the chart is an OCI chart
+func (st *HelmState) IsOCIChart(chart string) bool {
+	if strings.HasPrefix(chart, "oci://") {
+		return true
+	}
+
+	repo, _ := st.GetRepositoryAndNameFromChartName(chart)
+	if repo == nil {
+		return false
+	}
+	return repo.OCI
+}
+
 func (st *HelmState) getOCIQualifiedChartName(release *ReleaseSpec, helm helmexec.Interface) (qualifiedChartName, chartName, chartVersion string, err error) {
 	chartVersion = "latest"
 	if release.Version != "" {

pkg/state/state_test.go

@@ -4088,3 +4088,118 @@ func TestHelmState_chartOCIFlags(t *testing.T) {
 		})
 	}
 }
+
+func TestIsOCIChart(t *testing.T) {
+	cases := []struct {
+		st       *HelmState
+		chart    string
+		expected bool
+	}{
+		{&HelmState{}, "oci://myrepo/mychart", true},
+		{&HelmState{}, "oci://myrepo/mychart:1.0.0", true},
+		{&HelmState{}, "myrepo/mychart", false},
+		{&HelmState{}, "myrepo/mychart:1.0.0", false},
+		{
+			&HelmState{
+				ReleaseSetSpec: ReleaseSetSpec{
+					Repositories: []RepositorySpec{
+						{
+							Name: "ocirepo",
+							URL:  "ocirepo.com",
+							OCI:  true,
+						},
+					},
+				},
+			},
+			"ocirepo/chart",
+			true,
+		},
+		{
+			&HelmState{
+				ReleaseSetSpec: ReleaseSetSpec{
+					Repositories: []RepositorySpec{
+						{
+							Name: "nonocirepo",
+							URL:  "nonocirepo.com",
+						},
+					},
+				},
+			},
+			"nonocirepo/chart",
+			false,
+		},
+	}
+
+	for _, c := range cases {
+		actual := c.st.IsOCIChart(c.chart)
+		if actual != c.expected {
+			t.Errorf("IsOCIChart(%s) = %t; expected %t", c.chart, actual, c.expected)
+		}
+	}
+}
+
+func TestAppendVerifyFlags(t *testing.T) {
+	st := &HelmState{}
+
+	tests := []struct {
+		name         string
+		repo         []RepositorySpec
+		helmDefaults HelmSpec
+		release      *ReleaseSpec
+		expected     []string
+	}{
+		{
+			name:         "Release with true verify flag",
+			release:      &ReleaseSpec{Verify: boolValue(true)},
+			repo:         nil,
+			helmDefaults: HelmSpec{},
+			expected:     []string{"--verify"},
+		},
+		{
+			name:         "Release with false verify flag",
+			release:      &ReleaseSpec{Verify: boolValue(false)},
+			repo:         nil,
+			helmDefaults: HelmSpec{},
+			expected:     []string(nil),
+		},
+		{
+			name:         "Repository with verify flag",
+			helmDefaults: HelmSpec{},
+			repo: []RepositorySpec{
+				{
+					Name:   "myrepo",
+					Verify: true,
+				},
+			},
+			release: &ReleaseSpec{
+				Chart: "myrepo/mychart",
+			},
+			expected: []string{"--verify"},
+		},
+		{
+			name: "Helm defaults with verify flag",
+			repo: nil,
+			helmDefaults: HelmSpec{
+				Verify: true,
+			},
+			release:  &ReleaseSpec{},
+			expected: []string{"--verify"},
+		},
+		{
+			name:         "No verify flag",
+			repo:         nil,
+			helmDefaults: HelmSpec{},
+			release:      &ReleaseSpec{},
+			expected:     []string(nil),
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			st.ReleaseSetSpec.Repositories = tt.repo
+			st.ReleaseSetSpec.HelmDefaults = tt.helmDefaults
+			flags := st.appendVerifyFlags(nil, tt.release)
+			assert.Equal(t, tt.expected, flags)
+		})
+	}
+}