Support VPC security group for PowerVS clusters #1738

dharaneeshvrd · 2024-04-19T07:52:48Z

What this PR does / why we need it:
Add support for VPC security group for PowerVS clusters

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #
Fixes #1706

Special notes for your reviewer:

/area provider/ibmcloud

Please confirm that if this PR changes any image versions, then that's the sole change this PR makes.

Release note:

Support VPC security group for PowerVS clusters

netlify · 2024-04-19T07:53:06Z

✅ Deploy Preview for kubernetes-sigs-cluster-api-ibmcloud ready!

Name	Link
🔨 Latest commit	`687d1d6`
🔍 Latest deploy log	https://app.netlify.com/sites/kubernetes-sigs-cluster-api-ibmcloud/deploys/664c74e7addf3200085a229d
😎 Deploy Preview	https://deploy-preview-1738--kubernetes-sigs-cluster-api-ibmcloud.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Prajyot-Parab · 2024-04-19T16:53:03Z

/cc @Karthik-K-N

Karthik-K-N · 2024-04-22T05:31:03Z

/cc @cjschaef

k8s-ci-robot · 2024-04-22T05:31:05Z

@Karthik-K-N: GitHub didn't allow me to request PR reviews from the following users: cjschaef.

Note that only kubernetes-sigs members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

/cc @cjschaef

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Karthik-K-N · 2024-04-22T06:21:07Z

cloud/scope/powervs_cluster.go

+			return true, fmt.Errorf("error validating existing security group: %v", err)
+		}
+		if securityGroupID != nil {
+			s.Logger.Info("Security group already exists", "name", *securityGroup.Name)


securityGroup.Name can be nil, so its better to handle it everywhere before directly de referring it.

Refactored it to get it from vpcv1.SecurityGroup where name is a required param.

Karthik-K-N · 2024-04-22T06:22:24Z

cloud/scope/powervs_cluster.go

+				ControllerCreated: ptr.To(false),
+			})
+
+			return false, nil


I think in case of multiple security groups we need to continue to process next item instead of returning from here.

Yes, Done
thanks!

Karthik-K-N · 2024-04-22T06:23:41Z

cloud/scope/powervs_cluster.go

+		}
+
+		s.SetVPCSecurityGroupID(*securityGroup.Name, infrav1beta2.ResourceReference{
+			ID:                securityGroupID,


I think we should update the status as soon as we create rather than doing it after creating security group rule as it might fail.

Please have a look at the latest implementation, added ruleIDs also in status.

Karthik-K-N · 2024-04-22T06:47:43Z

cloud/scope/powervs_cluster.go

+
+	options := &vpcv1.CreateSecurityGroupOptions{
+		VPC: &vpcv1.VPCIdentity{
+			ID: s.GetVPCID(),


s.GetVPCID() can be nil, So lets handle it.

As per the reconcile order, when it reaches here VPC status would have been populated right?
Do we really need to check? If its not populated, it would requeue on ReconcileVPC only I believe.

Karthik-K-N · 2024-04-22T06:52:24Z

cloud/scope/powervs_cluster.go

+			if cidrSubnet == nil {
+				return fmt.Errorf("subnet by name '%s' not exist", *remote.CIDRSubnetName)
+			}
+			s.Logger.Info("Creating security group rule", "securityGroupID", *securityGroupID, "direction", *direction, "protocol", *protocol, "cidrBlockSubnet", *remote.CIDRSubnetName, "cidr", *cidrSubnet.Ipv4CIDRBlock)


May be lets use V level logs to dump detailed infos

Karthik-K-N · 2024-04-22T07:28:34Z

cloud/scope/powervs_cluster.go

+					return false, fmt.Errorf("error validating security group rule: %v", err)
+				}
+				if !match {
+					return false, nil


How about logging with V(3) with which security group rule did not match.

It's really difficult to get that since it's not that straight forward. Let's keep it like this for now!

Karthik-K-N · 2024-04-22T07:29:31Z

cloud/scope/powervs_cluster.go

+// validateSecurityGroupRule compares a specific security group's rule with the spec and existing security group's rule.
+func (s *PowerVSClusterScope) validateSecurityGroupRule(originalSecurityGroup vpcv1.SecurityGroup, direction infrav1beta2.SecurityGroupRuleDirection, protocol string, portMin, portMax int64, icmpCode, icmpType *string, remote infrav1beta2.SecurityGroupRuleRemote) (bool, error) {
+	var match bool
+	var err error


How about declaring them in return parameters?

Karthik-K-N · 2024-04-22T07:32:25Z

cloud/scope/powervs_cluster.go

+				}
+			}
+		}
+		if match {


Is this should be !match? because we should validate all the rules, If the first rule matches we should not return right?

Perhaps we do not return and attempt to validate only one rule matches.
I expect this function is attempting to verify only one SecurityGroupRule definition from Spec exists in the SecurityGroup. And the function (validateSecurityGroupRule) gets called once for each SecurityGroupRule defined (per SecurityGroup).

So exiting here makes sense IMO, if we only want to validate that a SG Rule exists (expecting we wouldn't have duplicates.....which I don't know if the API allows that).

Yes its trying to match that particular rule passed is exists or not. So exiting as soon as it finds a match.

Karthik-K-N · 2024-04-22T07:38:26Z

cloud/scope/powervs_cluster.go

+
+// validateVPCSecurityGroup validates the security group and it's rules.
+func (s *PowerVSClusterScope) validateVPCSecurityGroup(securityGroup infrav1beta2.SecurityGroup) (*string, error) {
+	securityGroupDet, err := s.IBMVPCClient.GetSecurityGroupByName(*securityGroup.Name)


May we can try to fetch by id as well, if id is already set in status? also we need to make sure name is not nil.

Yes agree, refactored, ptal!

cjschaef

The design makes sense, handling the complexities of SecurityGroups and SecurityGroupRules definitions.

In my mind, I was thinking VPC would iterate through creating all the SecurityGroups first, then return to populate the SecurityGroupRules, as some of the Remotes depend on the SecurityGroups (CRN). I will rethink the process some compared with the expectations here, if this looping is acceptable, when to raise errors, etc. due to these additional complexities (inter SecurityGroup/SecurityGroupRule dependencies).

cjschaef · 2024-04-23T21:49:41Z

cloud/scope/powervs_cluster.go

+		if _, _, err := s.IBMVPCClient.GetSecurityGroup(&vpcv1.GetSecurityGroupOptions{
+			ID: securityGroup.ID,
+		}); err != nil {
+			if strings.Contains(err.Error(), "not found") {


Doesn't the detailedResponse provide an Http StatusCode you could check rather than rely on the error text?

Yes it would, but this method of validating the resource deletion is followed through out this controller code.
IMO also it's fine.

cjschaef · 2024-04-23T22:03:41Z

pkg/cloud/services/vpc/service.go

+		if start != "" {
+			listSecurityGroupOptions.Start = &start
+		}
+		securityGroupCollection, _, err := s.ListSecurityGroups(&listSecurityGroupOptions)


Perhaps we instead use the vpcv1.SecurityGroupsPager functionality instead?

Thanks for the suggestion, implemented using pager.

cjschaef · 2024-04-23T22:11:43Z

cloud/scope/powervs_cluster.go

+				}
+			}
+		}
+		if match {


Perhaps we do not return and attempt to validate only one rule matches.
I expect this function is attempting to verify only one SecurityGroupRule definition from Spec exists in the SecurityGroup. And the function (validateSecurityGroupRule) gets called once for each SecurityGroupRule defined (per SecurityGroup).

So exiting here makes sense IMO, if we only want to validate that a SG Rule exists (expecting we wouldn't have duplicates.....which I don't know if the API allows that).

dharaneeshvrd · 2024-04-24T07:38:57Z

The design makes sense, handling the complexities of SecurityGroups and SecurityGroupRules definitions.

In my mind, I was thinking VPC would iterate through creating all the SecurityGroups first, then return to populate the SecurityGroupRules, as some of the Remotes depend on the SecurityGroups (CRN). I will rethink the process some compared with the expectations here, if this looping is acceptable, when to raise errors, etc. due to these additional complexities (inter SecurityGroup/SecurityGroupRule dependencies).

@cjschaef
I have n't used security groups that much, so could n't think of this use case. With the current implementation if we pass the security groups in order which the dependancy chain is, it would still handle it. But if we really need a freedom of referring the sg's CRN from different order, then we need to redesign and it will bring more complexities than current one.
Please let me know whether the current approach is ok or not!

Also I have refactored few things from last review, ptal!

dharaneeshvrd · 2024-04-24T07:41:16Z

/retest-required

Karthik-K-N

Thanks, Few questions otherwise, Overall LGTM

Karthik-K-N · 2024-04-25T04:57:19Z

cloud/scope/powervs_cluster.go

+	setRemote := func(remote infrav1beta2.SecurityGroupRuleRemote, remoteOption *vpcv1.SecurityGroupRuleRemotePrototype) error {
+		switch remote.RemoteType {
+		case infrav1beta2.SecurityGroupRuleRemoteTypeCIDR:
+			cidrSubnet, err := s.IBMVPCClient.GetVPCSubnetByName(*remote.CIDRSubnetName)


As of now do we have any check which mandates the user to set remote.CIDRSubnetName when type is infrav1beta2.SecurityGroupRuleRemoteTypeCIDR , If not lets create an issue to add a wehbook later

Yes this is validated in kubebuilder rules in spec.

Karthik-K-N · 2024-04-25T05:03:22Z

cloud/scope/powervs_cluster.go

+// ReconcileVPCSecurityGroup reconciles VPC security group.
+func (s *PowerVSClusterScope) ReconcileVPCSecurityGroup() (bool, error) {
+	for _, securityGroup := range s.IBMPowerVSCluster.Spec.VPCSecurityGroups {
+		securityGroupID, securityGroupRuleIDs := s.GetVPCSecurityGroupID(*securityGroup.Name)


Lets carefully handle *securityGroup.Name when user sets securityGroup.ID

Sure, taken care, please check!

Karthik-K-N

LGTM
@mkumatag @Prajyot-Parab @Amulyam24 Please take a look. Thanks

Amulyam24 · 2024-05-06T06:35:56Z

cloud/scope/powervs_cluster.go

+					SecurityGroupID: securityGroupID,
+					ID:              rule,
+				}); err != nil {
+					return true, err


@dharaneeshvrd, shouldn't we be returning false on err and not requeue?

The requeue was added for handling the state of the infra resources, maybe it can be skipped if security group doesn't need state management?

Yes Agree, corrected.
Thanks @Amulyam24 !

@dharaneesh, since we are returning false every time, maybe we can just return err and remove the requeue check in the controller?

Amulyam24 · 2024-05-07T08:15:08Z

cloud/scope/powervs_cluster.go

+			return false, fmt.Errorf("error validating existing security group: %v", err)
+		}
+		if securityGroupDet != nil {
+			s.Logger.Info("Security group already exists", "name", *securityGroupDet.Name)


Can we set log level to 3 wherever applicable?

I think its ok here, since its going to this block only one time. Same is there on other components too when user provided spec is already exists in cloud, wdys?

~~By default, we are setting the log level to 3 to keep uniformity as per the new changes in PR.~~
Refactoring the logs according to inputs shared here

Amulyam24 · 2024-05-07T08:17:57Z

cloud/scope/powervs_cluster.go

+		if _, _, err := s.IBMVPCClient.GetSecurityGroup(&vpcv1.GetSecurityGroupOptions{
+			ID: securityGroup.ID,
+		}); err != nil {
+			if strings.Contains(err.Error(), "not found") {


Can we add not found as a constant in types.go?

Amulyam24 · 2024-05-07T08:18:43Z

cloud/scope/powervs_cluster.go

+		}); err != nil {
+			if strings.Contains(err.Error(), "not found") {
+				s.Info("VPC security group successfully deleted", "ID", securityGroup.ID)
+				return false, nil


same comment applies here - please check if returning err would be enough and requeue can be avoided if not needed.

Yes true, I followed the vpc subnet deletion, but I think its bug to return from the loop after processing one item here ;)

Amulyam24

@dharaneeshvrd, couple of things, with log refactoring changes, do you mind following the below in this PR:

consistent naming for VPC and infra resources in logs such as VPC, VPC subnet, VPC security group etc.
Error messages starting with failed to...

Apart from minor comments, overall LGTM

Amulyam24 · 2024-05-08T06:00:06Z

cloud/scope/powervs_cluster.go

+			ControllerCreated: ptr.To(true),
+		})
+	}
+


Can we log a statement on successful creation of a security group?

dharaneeshvrd · 2024-05-09T05:57:31Z

@Amulyam24
I have refactored the logs and changed them to V(3).
Now I am not seeing any logs by default, let me know if you want to change the level here to display some if you planned, or please take care of them in your log refactor PR!
Thanks!

Amulyam24 · 2024-05-09T07:56:08Z

@Amulyam24 I have refactored the logs and changed them to V(3). Now I am not seeing any logs by default, let me know if you want to change the level here to display some if you planned, or please take care of them in your log refactor PR! Thanks!

Sure @dharaneeshvrd, I can take care of them in the logging PR once this is merged. Changes LGTM!
@Prajyot-Parab @mkumatag PTAL.

mkumatag · 2024-05-09T08:00:37Z

api/v1beta2/ibmpowervscluster_types.go

@@ -103,6 +103,10 @@ type IBMPowerVSClusterSpec struct {
 	// +optional
 	VPCSubnets []Subnet `json:"vpcSubnets,omitempty"`

+	// securityGroups to attach it to the VPC resource
+	// +optional
+	VPCSecurityGroups []SecurityGroup `json:"vpcSecurityGroups,omitempty"`


any specific reason for not naming this as VPCSecurityGroup?

Since its a list item, named it in plural just like VPCSubnets and LoadBalancers.

n/m, this SecurityGroup struct is created earlier in the types.go file

api/v1beta2/ibmpowervscluster_types.go

mkumatag · 2024-05-09T08:07:35Z

api/v1beta2/ibmvpccluster_types.go

+	// id represents the id of the resource.
+	ID *string `json:"id,omitempty"`
+	// rules contains the id of rules created under the security group
+	RuleIDs []*string `json:"rules,omitempty"`


Its always good to keep both the struct fieldname and the json name same.

mkumatag · 2024-05-09T08:11:39Z

cloud/scope/powervs_cluster.go

@@ -494,6 +496,44 @@ func (s *PowerVSClusterScope) SetVPCSubnetID(name string, resource infrav1beta2.
 	s.IBMPowerVSCluster.Status.VPCSubnet[name] = resource
 }

+// GetVPCSecurityGroupID returns the VPC security group id.
+func (s *PowerVSClusterScope) GetVPCSecurityGroupID(name string) (*string, []*string) {


function name says GetVPCSecurityGroupID but returning both security group id and also the rules.

Suggested change

func (s *PowerVSClusterScope) GetVPCSecurityGroupID(name string) (*string, []*string) {

func (s *PowerVSClusterScope) GetVPCSecurityGroup(name string) (*string, []*string) {

mkumatag · 2024-05-09T08:14:55Z

cloud/scope/powervs_cluster.go

+}
+
+// GetVPCSecurityGroupRuleIDs returns the VPC security group's ruleIDs.
+func (s *PowerVSClusterScope) GetVPCSecurityGroupRuleIDs(securityGroupID string) []*string {


this is more or less same as GetVPCSecurityGroupID but only difference is that we are supplying the id as param

we can keep the functions like GetVPCSecurityGroupByName and GetVPCSecurityGroupByID with returning same content.

mkumatag · 2024-05-09T10:00:55Z

cloud/scope/powervs_cluster.go

+}
+
+// validateVPCSecurityGroupRules compares a specific security group rules spec with the existing security group's rules.
+func (s *PowerVSClusterScope) validateVPCSecurityGroupRules(originalSecurityGroup *vpcv1.SecurityGroup, expectedSecurityGroup infrav1beta2.SecurityGroup) ([]*string, bool, error) {


If we care just about rules as a param, then lets send only rules as a param

ruleIDs, ok, err := s.validateVPCSecurityGroupRules(securityGroupDet, securityGroup.Rules)

This comment applies for other functions as well.

mkumatag · 2024-05-09T10:09:00Z

cloud/scope/powervs_cluster.go

+		switch direction {
+		case infrav1beta2.SecurityGroupRuleDirectionInbound:
+			protocol = string(expectedRule.Source.Protocol)
+			portMin = int64(expectedRule.Source.PortRange.MinimumPort)
+			portMax = int64(expectedRule.Source.PortRange.MaximumPort)
+			icmpCode := expectedRule.Source.ICMPCode
+			icmpType := expectedRule.Source.ICMPType
+
+			for _, remote := range expectedRule.Source.Remotes {
+				id, match, err := s.validateSecurityGroupRule(originalSecurityGroup, direction, protocol, portMin, portMax, icmpCode, icmpType, remote)
+				if err != nil {
+					return nil, false, fmt.Errorf("failed to validate security group rule: %w", err)
+				}
+				if !match {
+					return nil, false, nil
+				}
+				ruleIDs = append(ruleIDs, id)
+			}
+		case infrav1beta2.SecurityGroupRuleDirectionOutbound:
+			protocol = string(expectedRule.Destination.Protocol)
+			portMin = int64(expectedRule.Destination.PortRange.MinimumPort)
+			portMax = int64(expectedRule.Destination.PortRange.MaximumPort)
+			icmpCode := expectedRule.Destination.ICMPCode
+			icmpType := expectedRule.Destination.ICMPType
+
+			for _, remote := range expectedRule.Destination.Remotes {
+				id, match, err := s.validateSecurityGroupRule(originalSecurityGroup, direction, protocol, portMin, portMax, icmpCode, icmpType, remote)
+				if err != nil {
+					return nil, false, fmt.Errorf("failed to validate security group rule: %v", err)
+				}
+				if !match {
+					return nil, false, nil
+				}
+				ruleIDs = append(ruleIDs, id)
+			}
+		}


except few, there is lots of common code, wondering if we can reuse some part of it

applies to other functions as well.

tried to reuse some parts, ptal now!

mkumatag · 2024-05-09T10:18:09Z

cloud/scope/powervs_cluster.go

+			}
+		}
+		if match {
+			break


you can directly return from here instead.

mkumatag · 2024-05-09T10:27:56Z

cloud/scope/powervs_cluster.go

+			match = true
+		}
+	case infrav1beta2.SecurityGroupRuleRemoteTypeIP:
+		if originalSGRemote.Address != nil && *originalSGRemote.Address == *expectedSGRemote.IP {


why the field name is IP? do we need to rename this to Address

orig:

cluster-api-provider-ibmcloud/api/v1beta2/types.go

Line 344 in 0d56e7a

IP *string `json:"ip,omitempty"`

renamed to Address to keep it in sync with the VPC's field.

mkumatag · 2024-05-09T10:30:11Z

cloud/scope/powervs_cluster.go

+			return false, fmt.Errorf("failed to find vpc subnet by name '%s' for getting cidr block: %w", *expectedSGRemote.CIDRSubnetName, err)
+		}
+
+		if originalSGRemote.CIDRBlock != nil && cidrSubnet != nil && *originalSGRemote.CIDRBlock == *cidrSubnet.Ipv4CIDRBlock {


and also here

cluster-api-provider-ibmcloud/api/v1beta2/types.go

Line 344 in 0d56e7a

IP *string `json:"ip,omitempty"`

dharaneeshvrd · 2024-05-14T06:46:58Z

Addressed your comments, ptal now @mkumatag

Karthik-K-N

Overall LGTM

mkumatag

/lgtm

k8s-ci-robot · 2024-05-22T04:03:00Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dharaneeshvrd, mkumatag

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [mkumatag]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

k8s-ci-robot added area/provider/ibmcloud Issues or PRs related to ibmcloud provider cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Apr 19, 2024

k8s-ci-robot requested review from mkumatag and Prajyot-Parab April 19, 2024 07:52

k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Apr 19, 2024

dharaneeshvrd force-pushed the vpc-security-group branch 3 times, most recently from 218dc9c to fc926e4 Compare April 19, 2024 11:09

k8s-ci-robot requested a review from Karthik-K-N April 19, 2024 16:53

Karthik-K-N reviewed Apr 22, 2024

View reviewed changes

cjschaef reviewed Apr 23, 2024

View reviewed changes

dharaneeshvrd force-pushed the vpc-security-group branch 3 times, most recently from efc2b5f to 80acbd1 Compare April 24, 2024 07:05

dharaneeshvrd force-pushed the vpc-security-group branch 3 times, most recently from 2ed95eb to 11743b2 Compare April 24, 2024 09:37

Karthik-K-N reviewed Apr 25, 2024

View reviewed changes

dharaneeshvrd force-pushed the vpc-security-group branch from 11743b2 to fd93a8d Compare April 25, 2024 11:17

Karthik-K-N reviewed Apr 29, 2024

View reviewed changes

Amulyam24 reviewed May 6, 2024

View reviewed changes

dharaneeshvrd force-pushed the vpc-security-group branch from fd93a8d to 065ffc0 Compare May 6, 2024 08:02

Amulyam24 reviewed May 7, 2024

View reviewed changes

dharaneeshvrd force-pushed the vpc-security-group branch from 065ffc0 to d94ef4a Compare May 7, 2024 09:18

dharaneeshvrd force-pushed the vpc-security-group branch from d94ef4a to ad48c87 Compare May 7, 2024 09:25

Amulyam24 reviewed May 8, 2024

View reviewed changes

dharaneeshvrd force-pushed the vpc-security-group branch from ad48c87 to ad07a5b Compare May 9, 2024 05:53

dharaneeshvrd force-pushed the vpc-security-group branch from ad07a5b to 36ce8ea Compare May 9, 2024 05:59

mkumatag requested changes May 9, 2024

View reviewed changes

dharaneeshvrd force-pushed the vpc-security-group branch 7 times, most recently from 8f10554 to ddaf357 Compare May 14, 2024 06:39

dharaneeshvrd force-pushed the vpc-security-group branch 2 times, most recently from 3b1b413 to c417314 Compare May 16, 2024 13:41

mkumatag added this to the v0.8 milestone May 17, 2024

Karthik-K-N reviewed May 21, 2024

View reviewed changes

dharaneeshvrd force-pushed the vpc-security-group branch 2 times, most recently from abd7bea to e328d9c Compare May 21, 2024 10:09

Support VPC security group for PowerVS clusters

687d1d6

dharaneeshvrd force-pushed the vpc-security-group branch from e328d9c to 687d1d6 Compare May 21, 2024 10:18

mkumatag approved these changes May 22, 2024

View reviewed changes

k8s-ci-robot assigned mkumatag May 22, 2024

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 22, 2024

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 22, 2024

k8s-ci-robot merged commit b07b85a into kubernetes-sigs:main May 22, 2024
13 checks passed

	func (s PowerVSClusterScope) GetVPCSecurityGroupID(name string) (string, []*string) {
	func (s PowerVSClusterScope) GetVPCSecurityGroup(name string) (string, []*string) {

Support VPC security group for PowerVS clusters #1738

Support VPC security group for PowerVS clusters #1738

Conversation

dharaneeshvrd commented Apr 19, 2024 • edited

netlify bot commented Apr 19, 2024 • edited

✅ Deploy Preview for kubernetes-sigs-cluster-api-ibmcloud ready!

Prajyot-Parab commented Apr 19, 2024

Karthik-K-N commented Apr 22, 2024

k8s-ci-robot commented Apr 22, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cjschaef left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dharaneeshvrd Apr 24, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dharaneeshvrd commented Apr 24, 2024

dharaneeshvrd commented Apr 24, 2024

Karthik-K-N left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Karthik-K-N left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Amulyam24 May 6, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Amulyam24 May 8, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Amulyam24 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dharaneeshvrd commented May 9, 2024

Amulyam24 commented May 9, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dharaneeshvrd commented Apr 19, 2024 •

edited

netlify bot commented Apr 19, 2024 •

edited

dharaneeshvrd Apr 24, 2024 •

edited

Amulyam24 May 6, 2024 •

edited

Amulyam24 May 8, 2024 •

edited