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

PTEUDO-1602: DBRoleClaim Tests Update #374

Merged
merged 2 commits into from
Dec 16, 2024

Conversation

apulariravichandran
Copy link
Contributor

@apulariravichandran apulariravichandran commented Dec 6, 2024

Added the following new DbRoleClaim Tests , creating a roleclaim in new namespace for a Dbclaim existing in another namespace

Output of running test

STEP: Opening a connection to the database using the DSN from the secret in the new namespace 
Connection to the database successful
STEP: Running the query to get the tables in the new namespace 
Table: users, Owner: postgres, Size: 16 kB, Description:   
< Exit [It] should successfully reconcile the resource 
……..
[AfterSuite] PASSED [1.126 seconds]
------------------------------

Ran 2 of 11 Specs in 5.742 seconds
SUCCESS! -- 2 Passed | 0 Failed | 0 Pending | 9 Skipped
PASS
ok      github.com/infobloxopen/db-controller/internal/controller       6.667s

Expect(err).NotTo(HaveOccurred())

By("Creating the custom resource for the Kind DbRoleClaim in the new namespace")
dbroleclaimdiffnamespace := &persistancev1.DbRoleClaim{}
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's use camelCase for vars in Go.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

By("Getting the DbRoleClaim resource created in the new namespace")
dbroleclaimdiffnamespace := &persistancev1.DbRoleClaim{}
err = k8sClient.Get(ctx, types.NamespacedName{Name: resourceName, Namespace: newNamespace}, dbroleclaimdiffnamespace)
fmt.Printf("Status of DbRoleClaim: %+v", dbroleclaimdiffnamespace.Status)
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of printing the value, you can check if the status is what we expect.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

err = k8sClient.Get(ctx, secretName, secret)
Expect(err).NotTo(HaveOccurred())

fmt.Printf("Secret Name: %s, Secret Data: %s", secret.Name, secret.Data)
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

Comment on lines 411 to 413
if err == nil {
fmt.Println("Connection to the database successful")
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we don't need to print this, you are already checking whether there is an error or not.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

if tableDescription.Valid {
description = tableDescription.String
}
fmt.Printf("Table: %s, Owner: %s, Size: %s, Description: %s", tableName, tableOwner, tableSize, description)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it is more secure to check instead of printing the values.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

Copy link
Contributor

@leandrorichardtoledo leandrorichardtoledo left a comment

Choose a reason for hiding this comment

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

Approved ✔️ .

You can create a follow-up task to extend the test and include one for testing the creation/relation between the schemas and roles.

@apulariravichandran apulariravichandran merged commit 9577bbd into main Dec 16, 2024
3 checks passed
@apulariravichandran apulariravichandran deleted the PTEUDO-1602_integration_tests branch December 16, 2024 18:59
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.

2 participants