From 2f2c6d0af9d8a0cedd2132423b08da853c923140 Mon Sep 17 00:00:00 2001 From: Georgi Sabev Date: Tue, 7 Jan 2025 15:48:56 +0000 Subject: [PATCH 1/3] Introduce the `clean-bin` Makefile target It deletes the content of the `bin` directories on all levels Co-authored-by: Danail Branekov --- Makefile | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Makefile b/Makefile index 0a5707220..83763a1da 100644 --- a/Makefile +++ b/Makefile @@ -120,5 +120,8 @@ bin/yq: bin bin/setup-envtest: bin go install sigs.k8s.io/controller-runtime/tools/setup-envtest@latest +clean-bin: + rm -f **/bin/* + vendir-update-dependencies: bin/vendir vendir sync --chdir tests From ff4d08a0595a7c436faace38b02bbc7c95a7fda1 Mon Sep 17 00:00:00 2001 From: Danail Branekov Date: Tue, 7 Jan 2025 15:49:49 +0000 Subject: [PATCH 2/3] An app is staged when it has current droplet set Checking for the ready condition on the app to determine whether the app is staged does not make sense as during an app restart, the app becomes unready (but remains staged as it references a valid droplet) Co-authored-by: Georgi Sabev --- api/repositories/app_repository.go | 3 +- api/repositories/app_repository_test.go | 43 +++---------------------- 2 files changed, 6 insertions(+), 40 deletions(-) diff --git a/api/repositories/app_repository.go b/api/repositories/app_repository.go index fee36d561..feb5d223e 100644 --- a/api/repositories/app_repository.go +++ b/api/repositories/app_repository.go @@ -22,7 +22,6 @@ import ( "github.com/BooleanCat/go-functional/v2/it/itx" "github.com/google/uuid" corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/types" @@ -670,7 +669,7 @@ func cfAppToAppRecord(cfApp korifiv1alpha1.CFApp) AppRecord { CreatedAt: cfApp.CreationTimestamp.Time, UpdatedAt: getLastUpdatedTime(&cfApp), DeletedAt: golangTime(cfApp.DeletionTimestamp), - IsStaged: meta.IsStatusConditionTrue(cfApp.Status.Conditions, korifiv1alpha1.StatusConditionReady), + IsStaged: cfApp.Spec.CurrentDropletRef.Name != "", envSecretName: cfApp.Spec.EnvSecretName, vcapServiceSecretName: cfApp.Status.VCAPServicesSecretName, vcapAppSecretName: cfApp.Status.VCAPApplicationSecretName, diff --git a/api/repositories/app_repository_test.go b/api/repositories/app_repository_test.go index 2e8c061d1..e3455e1a6 100644 --- a/api/repositories/app_repository_test.go +++ b/api/repositories/app_repository_test.go @@ -23,7 +23,6 @@ import ( . "github.com/onsi/gomega/gstruct" gomega_types "github.com/onsi/gomega/types" corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" @@ -109,7 +108,7 @@ var _ = Describe("AppRepository", func() { Stack: cfApp.Spec.Lifecycle.Data.Stack, }, })) - Expect(app.IsStaged).To(BeFalse()) + Expect(app.IsStaged).To(BeTrue()) Expect(app.DeletedAt).To(BeNil()) Expect(app.Relationships()).To(Equal(map[string]string{ @@ -117,43 +116,11 @@ var _ = Describe("AppRepository", func() { })) }) - When("the app has staged condition true", func() { + When("the app has no current droplet set", func() { BeforeEach(func() { - cfApp.Status.Conditions = []metav1.Condition{{ - Type: korifiv1alpha1.StatusConditionReady, - Status: metav1.ConditionTrue, - LastTransitionTime: metav1.Now(), - Reason: "staged", - Message: "staged", - }} - Expect(k8sClient.Status().Update(ctx, cfApp)).To(Succeed()) - Eventually(func(g Gomega) { - app := korifiv1alpha1.CFApp{} - g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(cfApp), &app)).To(Succeed()) - g.Expect(app.Status.Conditions).NotTo(BeEmpty()) - }).Should(Succeed()) - }) - - It("sets IsStaged to true", func() { - Expect(getErr).ToNot(HaveOccurred()) - Expect(app.IsStaged).To(BeTrue()) - }) - }) - - When("the app has staged condition false", func() { - BeforeEach(func() { - meta.SetStatusCondition(&cfApp.Status.Conditions, metav1.Condition{ - Type: korifiv1alpha1.StatusConditionReady, - Status: metav1.ConditionFalse, - Reason: "appStaged", - Message: "", - }) - Expect(k8sClient.Status().Update(ctx, cfApp)).To(Succeed()) - Eventually(func(g Gomega) { - app := korifiv1alpha1.CFApp{} - g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(cfApp), &app)).To(Succeed()) - g.Expect(meta.IsStatusConditionFalse(app.Status.Conditions, korifiv1alpha1.StatusConditionReady)).To(BeTrue()) - }).Should(Succeed()) + Expect(k8s.PatchResource(ctx, k8sClient, cfApp, func() { + cfApp.Spec.CurrentDropletRef.Name = "" + })).To(Succeed()) }) It("sets IsStaged to false", func() { From afae96a1638a229848ebf8dd5d90aa37f99ae41c Mon Sep 17 00:00:00 2001 From: Danail Branekov Date: Tue, 7 Jan 2025 16:05:20 +0000 Subject: [PATCH 3/3] Smoke tests for listing resources The tests verify that * Authorised users can list resources * Unauthorised users get empty resources list but no error issue #3636 Co-authored-by: Georgi Sabev --- go.mod | 5 +-- go.sum | 2 ++ tests/helpers/flock.go | 33 +++++++++++++++++ tests/helpers/k8s.go | 2 ++ tests/smoke/list_test.go | 69 ++++++++++++++++++++++++++++++++++++ tests/smoke/run_task_test.go | 2 +- tests/smoke/suite_test.go | 33 +++++++++++++++++ 7 files changed, 143 insertions(+), 3 deletions(-) create mode 100644 tests/helpers/flock.go create mode 100644 tests/smoke/list_test.go diff --git a/go.mod b/go.mod index a7b61591e..03a260234 100644 --- a/go.mod +++ b/go.mod @@ -20,6 +20,7 @@ require ( github.com/go-logr/logr v1.4.2 github.com/go-logr/stdr v1.2.2 github.com/go-resty/resty/v2 v2.16.2 + github.com/gofrs/flock v0.12.1 github.com/golang-jwt/jwt v3.2.2+incompatible github.com/google/go-containerregistry v0.20.2 github.com/google/uuid v1.6.0 @@ -83,6 +84,8 @@ require ( go.opentelemetry.io/otel/trace v1.32.0 // indirect go.opentelemetry.io/proto/otlp v1.3.1 // indirect golang.org/x/exp v0.0.0-20240823005443-9b4947da3948 // indirect + golang.org/x/sys v0.29.0 // indirect + golang.org/x/term v0.28.0 // indirect google.golang.org/genproto/googleapis/api v0.0.0-20241104194629-dd2ea8efbc28 // indirect gopkg.in/evanphx/json-patch.v4 v4.12.0 // indirect reconciler.io/runtime v0.20.0 // indirect @@ -180,8 +183,6 @@ require ( golang.org/x/net v0.34.0 // indirect golang.org/x/oauth2 v0.24.0 // indirect golang.org/x/sync v0.10.0 // indirect - golang.org/x/sys v0.29.0 // indirect - golang.org/x/term v0.28.0 // indirect golang.org/x/time v0.7.0 // indirect golang.org/x/tools v0.29.0 // indirect gomodules.xyz/jsonpatch/v2 v2.4.0 // indirect diff --git a/go.sum b/go.sum index 2ca4343b8..9fc2fa193 100644 --- a/go.sum +++ b/go.sum @@ -220,6 +220,8 @@ github.com/go-task/slim-sprig/v3 v3.0.0 h1:sUs3vkvUymDpBKi3qH1YSqBQk9+9D/8M2mN1v github.com/go-task/slim-sprig/v3 v3.0.0/go.mod h1:W848ghGpv3Qj3dhTPRyJypKRiqCdHZiAzKg9hl15HA8= github.com/gobuffalo/flect v1.0.3 h1:xeWBM2nui+qnVvNM4S3foBhCAL2XgPU+a7FdpelbTq4= github.com/gobuffalo/flect v1.0.3/go.mod h1:A5msMlrHtLqh9umBSnvabjsMrCcCpAyzglnDvkbYKHs= +github.com/gofrs/flock v0.12.1 h1:MTLVXXHf8ekldpJk3AKicLij9MdwOWkZ+a/jHHZby9E= +github.com/gofrs/flock v0.12.1/go.mod h1:9zxTsyu5xtJ9DK+1tFZyibEV7y3uwDxPPfbxeeHCoD0= github.com/gogo/protobuf v1.1.1/go.mod h1:r8qH/GZQm5c6nD/R0oafs1akxWv10x8SbQlK7atdtwQ= github.com/gogo/protobuf v1.2.1/go.mod h1:hp+jE20tsWTFYpLwKvXlhS1hjn+gTNwPg2I6zVXpSg4= github.com/gogo/protobuf v1.3.2 h1:Ov1cvc58UF3b5XjBnZv7+opcTcQFZebYjWzi34vdm4Q= diff --git a/tests/helpers/flock.go b/tests/helpers/flock.go new file mode 100644 index 000000000..edfcf3b04 --- /dev/null +++ b/tests/helpers/flock.go @@ -0,0 +1,33 @@ +package helpers + +import ( + "github.com/gofrs/flock" + . "github.com/onsi/ginkgo/v2" //lint:ignore ST1001 this is a test file + . "github.com/onsi/gomega" //lint:ignore ST1001 this is a test file +) + +type FLock struct { + lock *flock.Flock +} + +func NewFlock(lockFilePath string) *FLock { + return &FLock{ + lock: flock.New(lockFilePath), + } +} + +func (f *FLock) Execute(fn func()) { + GinkgoHelper() + + Eventually(func(g Gomega) { + ok, err := f.lock.TryLock() + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(ok).To(BeTrue()) + }).Should(Succeed()) + + defer func() { + Expect(f.lock.Unlock()).To(Succeed()) + }() + + fn() +} diff --git a/tests/helpers/k8s.go b/tests/helpers/k8s.go index dbc148e92..a5c80d2dc 100644 --- a/tests/helpers/k8s.go +++ b/tests/helpers/k8s.go @@ -41,6 +41,8 @@ func AddUserToKubeConfig(userName, userToken string) { } func RemoveUserFromKubeConfig(userName string) { + GinkgoHelper() + configAccess := clientcmd.NewDefaultPathOptions() config, err := configAccess.GetStartingConfig() Expect(err).NotTo(HaveOccurred()) diff --git a/tests/smoke/list_test.go b/tests/smoke/list_test.go new file mode 100644 index 000000000..d51097223 --- /dev/null +++ b/tests/smoke/list_test.go @@ -0,0 +1,69 @@ +package smoke_test + +import ( + "code.cloudfoundry.org/korifi/tests/helpers" + . "code.cloudfoundry.org/korifi/tests/matchers" + "github.com/google/uuid" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + . "github.com/onsi/gomega/gexec" + "github.com/onsi/gomega/types" +) + +var _ = Describe("list", func() { + listResources := func(resourceType string, resourcesMatch types.GomegaMatcher) { + cfCurlOutput, err := sessionOutput(helpers.Cf("curl", "/v3/"+resourceType)) + Expect(err).NotTo(HaveOccurred()) + Expect(cfCurlOutput).To(MatchJSONPath("$.resources", resourcesMatch)) + } + + BeforeEach(func() { + Expect(helpers.Cf("run-task", sharedData.BuildpackAppName, "-c", "sleep 120")).To(Exit(0)) + + upsiName := uuid.NewString() + Expect(helpers.Cf("create-user-provided-service", upsiName)).To(Exit(0)) + Expect(helpers.Cf("bind-service", sharedData.BuildpackAppName, upsiName)).To(Exit(0)) + }) + + DescribeTable("authorised users get the resources", + listResources, + Entry("apps", "apps", Not(BeEmpty())), + Entry("packages", "packages", Not(BeEmpty())), + Entry("processes", "processes", Not(BeEmpty())), + Entry("routes", "routes", Not(BeEmpty())), + Entry("service_instances", "service_instances", Not(BeEmpty())), + Entry("service_credential_bindings", "service_credential_bindings", Not(BeEmpty())), + Entry("tasks", "tasks", Not(BeEmpty())), + ) + + When("the user is not allowed to list", func() { + BeforeEach(func() { + serviceAccountFactory := helpers.NewServiceAccountFactory(sharedData.RootNamespace) + userName := uuid.NewString() + userToken := serviceAccountFactory.CreateServiceAccount(userName) + helpers.NewFlock(sharedData.FLockPath).Execute(func() { + helpers.AddUserToKubeConfig(userName, userToken) + }) + + DeferCleanup(func() { + helpers.NewFlock(sharedData.FLockPath).Execute(func() { + helpers.RemoveUserFromKubeConfig(userName) + }) + serviceAccountFactory.DeleteServiceAccount(userName) + }) + + Expect(helpers.Cf("auth", userName)).To(Exit(0)) + }) + + DescribeTable("unauthorised users get empty resources list", + listResources, + Entry("apps", "apps", BeEmpty()), + Entry("packages", "packages", BeEmpty()), + Entry("processes", "processes", BeEmpty()), + Entry("routes", "routes", BeEmpty()), + Entry("service_instances", "service_instances", BeEmpty()), + Entry("service_credential_bindings", "service_credential_bindings", BeEmpty()), + Entry("tasks", "tasks", BeEmpty()), + ) + }) +}) diff --git a/tests/smoke/run_task_test.go b/tests/smoke/run_task_test.go index cbc9edaa2..9157c282a 100644 --- a/tests/smoke/run_task_test.go +++ b/tests/smoke/run_task_test.go @@ -10,6 +10,6 @@ import ( var _ = Describe("cf run-task", func() { It("succeeds", func() { - Eventually(helpers.Cf("run-task", sharedData.BuildpackAppName, "-c", `echo "Hello from the task"`)).Should(Exit(0)) + Expect(helpers.Cf("run-task", sharedData.BuildpackAppName, "-c", `echo "Hello from the task"`)).To(Exit(0)) }) }) diff --git a/tests/smoke/suite_test.go b/tests/smoke/suite_test.go index fbf5b0f56..1d0f062b2 100644 --- a/tests/smoke/suite_test.go +++ b/tests/smoke/suite_test.go @@ -6,6 +6,8 @@ import ( "encoding/json" "fmt" "net/http" + "os" + "path/filepath" "strings" "testing" @@ -38,6 +40,7 @@ type SmokeTestSharedData struct { BuildpackAppName string `json:"buildpack_app_name"` DockerAppName string `json:"docker_app_name"` BrokerURL string `json:"broker_url"` + FLockPath string `json:"f_lock_path"` } var sharedData SmokeTestSharedData @@ -59,6 +62,11 @@ func TestSmoke(t *testing.T) { } var _ = SynchronizedBeforeSuite(func() []byte { + setCFHome(GinkgoParallelProcess()) + + lockDir, err := os.MkdirTemp("", "") + Expect(err).NotTo(HaveOccurred()) + sharedData = SmokeTestSharedData{ CfAdmin: uuid.NewString(), RootNamespace: helpers.GetDefaultedEnvVar("ROOT_NAMESPACE", "cf"), @@ -68,6 +76,7 @@ var _ = SynchronizedBeforeSuite(func() []byte { AppsDomain: helpers.GetRequiredEnvVar("APP_FQDN"), BuildpackAppName: uuid.NewString(), DockerAppName: uuid.NewString(), + FLockPath: filepath.Join(lockDir, "lock"), } serviceAccountFactory := helpers.NewServiceAccountFactory(sharedData.RootNamespace) @@ -105,14 +114,38 @@ var _ = SynchronizedBeforeSuite(func() []byte { var _ = SynchronizedAfterSuite(func() { }, func() { + setCFHome(GinkgoParallelProcess()) + + Expect(helpers.Cf("api", helpers.GetRequiredEnvVar("API_SERVER_ROOT"), "--skip-ssl-validation")).To(Exit(0)) + Expect(helpers.Cf("auth", sharedData.CfAdmin)).To(Exit(0)) + Expect(helpers.Cf("delete-org", sharedData.OrgName, "-f").Wait()).To(Exit()) Expect(helpers.Cf("delete-org", sharedData.BrokerOrgName, "-f").Wait()).To(Exit()) serviceAccountFactory := helpers.NewServiceAccountFactory(sharedData.RootNamespace) serviceAccountFactory.DeleteServiceAccount(sharedData.CfAdmin) helpers.RemoveUserFromKubeConfig(sharedData.CfAdmin) + + Expect(os.RemoveAll(filepath.Dir(sharedData.FLockPath))).To(Succeed()) }) +var _ = BeforeEach(func() { + setCFHome(GinkgoParallelProcess()) + + Expect(helpers.Cf("api", helpers.GetRequiredEnvVar("API_SERVER_ROOT"), "--skip-ssl-validation")).To(Exit(0)) + Expect(helpers.Cf("auth", sharedData.CfAdmin)).To(Exit(0)) + Expect(helpers.Cf("target", "-o", sharedData.OrgName, "-s", sharedData.SpaceName)).To(Exit(0)) +}) + +func setCFHome(ginkgoNode int) { + cfHomeDir, err := os.MkdirTemp("", fmt.Sprintf("ginkgo-%d", ginkgoNode)) + Expect(err).NotTo(HaveOccurred()) + DeferCleanup(func() { + Expect(os.RemoveAll(cfHomeDir)).To(Succeed()) + }) + os.Setenv("CF_HOME", cfHomeDir) +} + func sessionOutput(session *Session) (string, error) { if session.ExitCode() != 0 { return "", fmt.Errorf("Session %v exited with exit code %d: %s",