Skip to content

Commit 65558c0

Browse files
ppapapetrou76ppapapetrou76
committed
fix validateDestination
1 parent 0657778 commit 65558c0

File tree

10 files changed

+31
-72
lines changed

10 files changed

+31
-72
lines changed

‎acr_controller/application/client.go‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ type httpApplicationClient struct {
2323
httpClient *http.Client
2424
baseURL string
2525
token string
26-
rootpath string
2726
}
2827

2928
func NewHTTPApplicationClient(token string, address string, rootpath string) ApplicationClient {
@@ -47,14 +46,13 @@ func NewHTTPApplicationClient(token string, address string, rootpath string) App
4746
TLSClientConfig: &tls.Config{InsecureSkipVerify: true},
4847
},
4948
},
50-
baseURL: address,
51-
token: token,
52-
rootpath: rootpath,
49+
baseURL: address,
50+
token: token,
5351
}
5452
}
5553

5654
func (c *httpApplicationClient) execute(ctx context.Context, url string, result any) error {
57-
req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil)
55+
req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, http.NoBody)
5856
if err != nil {
5957
return err
6058
}

‎acr_controller/controller/controller.go‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,6 @@ import (
1313
appclientset "github.com/argoproj/argo-cd/v3/pkg/client/clientset/versioned"
1414

1515
appv1 "github.com/argoproj/argo-cd/v3/pkg/apis/application/v1alpha1"
16-
applisters "github.com/argoproj/argo-cd/v3/pkg/client/listers/application/v1alpha1"
17-
servercache "github.com/argoproj/argo-cd/v3/server/cache"
1816
)
1917

2018
var watchAPIBufferSize = 1000

‎acr_controller/server.go‎

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,11 @@ package acr_controller
22

33
import (
44
"context"
5-
"crypto/tls"
65
"fmt"
76
"net"
87
"net/http"
98
"time"
109

11-
applisters "github.com/argoproj/argo-cd/v3/pkg/client/listers/application/v1alpha1"
12-
settings_util "github.com/argoproj/argo-cd/v3/util/settings"
1310
"github.com/redis/go-redis/v9"
1411
log "github.com/sirupsen/logrus"
1512
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -36,15 +33,11 @@ var backoff = wait.Backoff{
3633
type ACRServer struct {
3734
ACRServerOpts
3835

39-
settings *settings_util.ArgoCDSettings
40-
log *log.Entry
4136
appInformer cache.SharedIndexInformer
42-
appLister applisters.ApplicationLister
4337
applicationClientset appclientset.Interface
4438

4539
// stopCh is the channel which when closed, will shutdown the Event Reporter server
46-
stopCh chan struct{}
47-
serviceSet *ACRServerSet
40+
stopCh chan struct{}
4841
}
4942

5043
type ACRServerSet struct{}
@@ -163,10 +156,6 @@ func (a *ACRServer) Listen() (*Listeners, error) {
163156
// golang/protobuf).
164157
func (a *ACRServer) Run(ctx context.Context, lns *Listeners) {
165158
httpS := a.newHTTPServer(ctx, a.ListenPort)
166-
tlsConfig := tls.Config{}
167-
tlsConfig.GetCertificate = func(_ *tls.ClientHelloInfo) (*tls.Certificate, error) {
168-
return a.settings.Certificate, nil
169-
}
170159
go func() { a.checkServeErr("httpS", httpS.Serve(lns.Main)) }()
171160
go a.RunController(ctx)
172161

@@ -187,13 +176,10 @@ func NewApplicationChangeRevisionServer(_ context.Context, opts ACRServerOpts) *
187176
appFactory := appinformer.NewSharedInformerFactoryWithOptions(opts.AppClientset, 0, appinformer.WithNamespace(appInformerNs), appinformer.WithTweakListOptions(func(_ *metav1.ListOptions) {}))
188177

189178
appInformer := appFactory.Argoproj().V1alpha1().Applications().Informer()
190-
appLister := appFactory.Argoproj().V1alpha1().Applications().Lister()
191179

192180
server := &ACRServer{
193181
ACRServerOpts: opts,
194-
log: log.NewEntry(log.StandardLogger()),
195182
appInformer: appInformer,
196-
appLister: appLister,
197183
applicationClientset: opts.AppClientset,
198184
}
199185

‎acr_controller/service/acr_service_test.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -229,22 +229,22 @@ func Test_getRevisions(r *testing.T) {
229229
r.Run("history list is empty", func(t *testing.T) {
230230
acrService := newTestACRService(&mocks.ApplicationClient{})
231231
current, previous := acrService.getRevisions(r.Context(), createTestApp(fakeApp))
232-
assert.Equal(t, "", current)
233-
assert.Equal(t, "", previous)
232+
assert.Empty(t, current)
233+
assert.Empty(t, previous)
234234
})
235235

236236
r.Run("history list is empty, but operation happens right now", func(t *testing.T) {
237237
acrService := newTestACRService(&mocks.ApplicationClient{})
238238
current, previous := acrService.getRevisions(r.Context(), createTestApp(fakeAppWithOperation))
239239
assert.Equal(t, "c732f4d2ef24c7eeb900e9211ff98f90bb646505", current)
240-
assert.Equal(t, "", previous)
240+
assert.Empty(t, previous)
241241
})
242242

243243
r.Run("history list contains only one element, also sync result is here", func(t *testing.T) {
244244
acrService := newTestACRService(&mocks.ApplicationClient{})
245245
current, previous := acrService.getRevisions(r.Context(), createTestApp(syncedAppWithSingleHistory))
246246
assert.Equal(t, "c732f4d2ef24c7eeb900e9211ff98f90bb646505", current)
247-
assert.Equal(t, "", previous)
247+
assert.Empty(t, previous)
248248
})
249249

250250
r.Run("application is synced", func(t *testing.T) {

‎cmd/application-change-revision-controller/commands/application_change_revision_controller.go‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -128,13 +128,9 @@ func NewCommand() *cobra.Command {
128128
lns, err := changeRevisionServer.Listen()
129129
errors.CheckError(err)
130130
for {
131-
var closer func()
132131
ctx, cancel := context.WithCancel(ctx)
133132
changeRevisionServer.Run(ctx, lns)
134133
cancel()
135-
if closer != nil {
136-
closer()
137-
}
138134
}
139135
},
140136
}

‎server/application/application_rollout_rollback.go‎

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -78,22 +78,22 @@ func (s *Server) getRsOfSpecificRevision(ctx context.Context, config *rest.Confi
7878
if err != nil {
7979
return nil, fmt.Errorf("error getting resource: %w", err)
8080
}
81-
if v := resource.GetRevision(rsliveObj); err == nil {
82-
if toRevision == 0 {
83-
if latestRevision < v {
84-
// newest one we've seen so far
85-
previousRevision = latestRevision
86-
previousReplicaSet = latestReplicaSet
87-
latestRevision = v
88-
latestReplicaSet = rsliveObj
89-
} else if previousRevision < v {
90-
// second newest one we've seen so far
91-
previousRevision = v
92-
previousReplicaSet = rsliveObj
93-
}
94-
} else if toRevision == v {
95-
return rsliveObj, nil
81+
v := resource.GetRevision(rsliveObj)
82+
switch toRevision {
83+
case 0:
84+
if latestRevision < v {
85+
// newest one we've seen so far
86+
previousRevision = latestRevision
87+
previousReplicaSet = latestReplicaSet
88+
latestRevision = v
89+
latestReplicaSet = rsliveObj
90+
} else if previousRevision < v {
91+
// second newest one we've seen so far
92+
previousRevision = v
93+
previousReplicaSet = rsliveObj
9694
}
95+
case v:
96+
return rsliveObj, nil
9797
}
9898
}
9999

@@ -138,7 +138,7 @@ func (s *Server) getReplicaSetForRolloutRollack(ctx context.Context, config *res
138138
rolloutGVK := getRolloutGVK()
139139

140140
foundRolloutNode := tree.FindNode(rolloutGVK.Group, rolloutGVK.Kind, q.GetRolloutNamespace(), q.GetRolloutName())
141-
if foundRolloutNode == nil || foundRolloutNode.ResourceRef.UID == "" {
141+
if foundRolloutNode == nil || foundRolloutNode.UID == "" {
142142
return nil, status.Errorf(codes.InvalidArgument, "%s %s %s not found as part of application %s", rolloutGVK.Kind, rolloutGVK.Group, q.GetRolloutName(), q.GetName())
143143
}
144144

‎server/application/application_validate_src_and_dest.go‎

Lines changed: 3 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,6 @@ import (
55
"errors"
66
"fmt"
77

8-
"google.golang.org/grpc/codes"
9-
"google.golang.org/grpc/status"
108
apierrors "k8s.io/apimachinery/pkg/api/errors"
119

1210
"github.com/argoproj/argo-cd/v3/pkg/apiclient/application"
@@ -37,7 +35,7 @@ func (s *Server) ValidateSrcAndDst(ctx context.Context, requset *application.App
3735

3836
if err := validateDestination(ctx, &app.Spec.Destination, s.db); err != nil {
3937
entity := destinationEntity
40-
errMsg := fmt.Sprintf("application destination spec for %s is invalid: %s", app.ObjectMeta.Name, err.Error())
38+
errMsg := fmt.Sprintf("application destination spec for %s is invalid: %s", app.Name, err.Error())
4139
return &application.ApplicationValidateResponse{
4240
Error: &errMsg,
4341
Entity: &entity,
@@ -55,7 +53,7 @@ func (s *Server) ValidateSrcAndDst(ctx context.Context, requset *application.App
5553
}
5654
if len(conditions) > 0 {
5755
entity := sourceEntity
58-
errMsg := fmt.Sprintf("application spec for %s is invalid: %s", app.ObjectMeta.Name, argo.FormatAppConditions(conditions))
56+
errMsg := fmt.Sprintf("application spec for %s is invalid: %s", app.Name, argo.FormatAppConditions(conditions))
5957
return &application.ApplicationValidateResponse{
6058
Error: &errMsg,
6159
Entity: &entity,
@@ -73,22 +71,5 @@ func validateDestination(ctx context.Context, dest *appv1.ApplicationDestination
7371
return errors.New("no destination defined in app spec")
7472
}
7573
_, err := argo.GetDestinationCluster(ctx, *dest, db)
76-
if err != nil {
77-
return err
78-
}
79-
80-
if dest.Server != "" {
81-
// Ensure the k8s cluster the app is referencing, is configured in Argo CD
82-
_, err := db.GetCluster(ctx, dest.Server)
83-
if err != nil {
84-
if errStatus, ok := status.FromError(err); ok && errStatus.Code() == codes.NotFound {
85-
return fmt.Errorf("cluster '%s' has not been configured", dest.Server)
86-
}
87-
return err
88-
}
89-
} else if dest.Server == "" {
90-
return errors.New("destination server missing from app spec")
91-
}
92-
93-
return nil
74+
return err
9475
}

‎server/application/cf_application.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import (
1212
appv1 "github.com/argoproj/argo-cd/v3/pkg/apis/application/v1alpha1"
1313
"github.com/argoproj/argo-cd/v3/reposerver/apiclient"
1414
"github.com/argoproj/argo-cd/v3/util/app/path"
15-
ioutil "github.com/argoproj/argo-cd/v3/util/io"
15+
utilio "github.com/argoproj/argo-cd/v3/util/io"
1616
)
1717

1818
const (
@@ -41,7 +41,7 @@ func (s *Server) GetChangeRevision(ctx context.Context, in *application.ChangeRe
4141
if err != nil {
4242
return nil, fmt.Errorf("error creating repo server client: %w", err)
4343
}
44-
defer ioutil.Close(closer)
44+
defer utilio.Close(closer)
4545

4646
response, err := client.GetChangeRevision(ctx, &apiclient.ChangeRevisionRequest{
4747
AppName: in.GetAppName(),

‎util/kustomize/repospec.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ func parseGitURL(n string) (
3434
index := strings.Index(n, gitSuffix)
3535
orgRepo = n[0:index]
3636
n = n[index+len(gitSuffix):]
37-
if len(n) > 0 && n[0] == '/' {
37+
if n != "" && n[0] == '/' {
3838
n = n[1:]
3939
}
4040
path, gitRef = peelQuery(n)

‎util/kustomize/repospec_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ var hostNamesRawAndNormalized = [][]string{
4343
}
4444

4545
func makeURL(hostFmt, orgRepo, path, href string) string {
46-
if len(path) > 0 {
46+
if path != "" {
4747
orgRepo = filepath.Join(orgRepo, path)
4848
}
4949
url := hostFmt + orgRepo

0 commit comments

Comments
 (0)