You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@fineract.apache.org by GitBox <gi...@apache.org> on 2021/01/18 11:29:11 UTC

[GitHub] [fineract] BLasan opened a new pull request #1583: Fineract 1294

BLasan opened a new pull request #1583:
URL: https://github.com/apache/fineract/pull/1583


   ## Description
   
   See [FINERACT-1294
   ](https://issues.apache.org/jira/browse/FINERACT-1294)
   
   ## Checklist
   
   Please make sure these boxes are checked before submitting your pull request - thanks!
   
   - [x] Write the commit message as per https://github.com/apache/fineract/#pull-requests
   
   - [x] Acknowledge that we will not review PRs that are not passing the build _("green")_ - it is your responsibility to get a proposed PR to pass the build, not primarily the project's maintainers.
   
   - [ ] Create/update unit or integration tests for verifying the changes made.
   
   - [x] Follow coding conventions at https://cwiki.apache.org/confluence/display/FINERACT/Coding+Conventions.
   
   - [ ] Add required Swagger annotation and update API documentation at fineract-provider/src/main/resources/static/api-docs/apiLive.htm with details of any API changes
   
   - [x] Submission is not a "code dump".  (Large changes can be made "in repository" via a branch.  Ask on the developer mailing list for guidance, if required.)
   
   FYI our guidelines for code reviews are at https://cwiki.apache.org/confluence/display/FINERACT/Code+Review+Guide.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [fineract] vorburger commented on a change in pull request #1583: Fix MYSQL Restart Issue

Posted by GitBox <gi...@apache.org>.
vorburger commented on a change in pull request #1583:
URL: https://github.com/apache/fineract/pull/1583#discussion_r559530662



##########
File path: kubernetes/kubectl-startup.sh
##########
@@ -19,7 +19,7 @@
 #
 
 echo "Setting Up Fineract service configuration..."
-kubectl create secret generic fineract-tenants-db-secret --from-literal=username=root --from-literal=password=$(head /dev/urandom | tr -dc A-Za-z0-9 | head -c 16)
+kubectl create secret generic fineract-tenants-db-secret --from-literal=username=root --from-literal=password=root

Review comment:
       Wait a second... doesn't this hardcode it to"root"?! That's..  probably not such a good idea? 😎




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [fineract] BLasan commented on a change in pull request #1583: Fix MYSQL Restart Issue

Posted by GitBox <gi...@apache.org>.
BLasan commented on a change in pull request #1583:
URL: https://github.com/apache/fineract/pull/1583#discussion_r559565337



##########
File path: kubernetes/kubectl-startup.sh
##########
@@ -19,7 +19,7 @@
 #
 
 echo "Setting Up Fineract service configuration..."
-kubectl create secret generic fineract-tenants-db-secret --from-literal=username=root --from-literal=password=$(head /dev/urandom | tr -dc A-Za-z0-9 | head -c 16)
+kubectl create secret generic fineract-tenants-db-secret --from-literal=username=root --from-literal=password=root

Review comment:
       @vorburger Please review the new changes




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [fineract] BLasan commented on pull request #1583: Fix MYSQL Access Denied Issue on restart

Posted by GitBox <gi...@apache.org>.
BLasan commented on pull request #1583:
URL: https://github.com/apache/fineract/pull/1583#issuecomment-764936401


   @vorburger Sir could you please take a look?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [fineract] vorburger merged pull request #1583: Fix MYSQL Access Denied Issue on restart

Posted by GitBox <gi...@apache.org>.
vorburger merged pull request #1583:
URL: https://github.com/apache/fineract/pull/1583


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [fineract] BLasan commented on a change in pull request #1583: Fix MYSQL Restart Issue

Posted by GitBox <gi...@apache.org>.
BLasan commented on a change in pull request #1583:
URL: https://github.com/apache/fineract/pull/1583#discussion_r559539660



##########
File path: kubernetes/kubectl-startup.sh
##########
@@ -19,7 +19,7 @@
 #
 
 echo "Setting Up Fineract service configuration..."
-kubectl create secret generic fineract-tenants-db-secret --from-literal=username=root --from-literal=password=$(head /dev/urandom | tr -dc A-Za-z0-9 | head -c 16)
+kubectl create secret generic fineract-tenants-db-secret --from-literal=username=root --from-literal=password=root

Review comment:
       Actually these usernames and passwords will be saved as secrets.  But anyway you can get those values by running the command `kubectl get secrets/fineract-tenants-db-secret --template={{.data.password}} | base64 --decode` . And the problem in here is that when we use a random password, after restarting the container we'll be getting access denied issue. 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [fineract] BLasan commented on a change in pull request #1583: Fix MYSQL Restart Issue

Posted by GitBox <gi...@apache.org>.
BLasan commented on a change in pull request #1583:
URL: https://github.com/apache/fineract/pull/1583#discussion_r559539660



##########
File path: kubernetes/kubectl-startup.sh
##########
@@ -19,7 +19,7 @@
 #
 
 echo "Setting Up Fineract service configuration..."
-kubectl create secret generic fineract-tenants-db-secret --from-literal=username=root --from-literal=password=$(head /dev/urandom | tr -dc A-Za-z0-9 | head -c 16)
+kubectl create secret generic fineract-tenants-db-secret --from-literal=username=root --from-literal=password=root

Review comment:
       Actually these usernames and passwords will be saved as secrets.  But anyway you can get those values by running the command `kubectl get secrets/fineract-tenants-db-secret --template={{.data.password}} | base64 --decode` . And the problem is that when we use a random password, after restarting the container we'll be getting access denied issue. 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [fineract] vorburger commented on a change in pull request #1583: Fix MYSQL Access Denied Issue on restart

Posted by GitBox <gi...@apache.org>.
vorburger commented on a change in pull request #1583:
URL: https://github.com/apache/fineract/pull/1583#discussion_r562188078



##########
File path: kubernetes/fineractmysql-deployment.yml
##########
@@ -96,11 +96,7 @@ spec:
                   key: password
           livenessProbe:
             exec:
-              command:
-                - mysqladmin
-                - ping
-                - -h
-                - localhost
+              command: ["sh","-c","mysqladmin ping -h localhost -uroot -p${MYSQL_ROOT_PASSWORD}"]

Review comment:
       @BLasan sorry I'm not getting something here- where/by what is this MYSQL_ROOT_PASSWORD defined? Tx. /Cc @xurror 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [fineract] BLasan commented on a change in pull request #1583: Fix MYSQL Restart Issue

Posted by GitBox <gi...@apache.org>.
BLasan commented on a change in pull request #1583:
URL: https://github.com/apache/fineract/pull/1583#discussion_r559565337



##########
File path: kubernetes/kubectl-startup.sh
##########
@@ -19,7 +19,7 @@
 #
 
 echo "Setting Up Fineract service configuration..."
-kubectl create secret generic fineract-tenants-db-secret --from-literal=username=root --from-literal=password=$(head /dev/urandom | tr -dc A-Za-z0-9 | head -c 16)
+kubectl create secret generic fineract-tenants-db-secret --from-literal=username=root --from-literal=password=root

Review comment:
       @vorburger Please review the new changes




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [fineract] BLasan commented on a change in pull request #1583: Fix MYSQL Restart Issue

Posted by GitBox <gi...@apache.org>.
BLasan commented on a change in pull request #1583:
URL: https://github.com/apache/fineract/pull/1583#discussion_r559542254



##########
File path: kubernetes/kubectl-startup.sh
##########
@@ -19,7 +19,7 @@
 #
 
 echo "Setting Up Fineract service configuration..."
-kubectl create secret generic fineract-tenants-db-secret --from-literal=username=root --from-literal=password=$(head /dev/urandom | tr -dc A-Za-z0-9 | head -c 16)
+kubectl create secret generic fineract-tenants-db-secret --from-literal=username=root --from-literal=password=root

Review comment:
       I do have another solution for this. Not sure whether it'll help to sort out this issue. Let me try that way as well :)




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [fineract] vorburger commented on a change in pull request #1583: Fix MYSQL Access Denied Issue on restart

Posted by GitBox <gi...@apache.org>.
vorburger commented on a change in pull request #1583:
URL: https://github.com/apache/fineract/pull/1583#discussion_r563161080



##########
File path: kubernetes/fineractmysql-deployment.yml
##########
@@ -96,11 +96,7 @@ spec:
                   key: password
           livenessProbe:
             exec:
-              command:
-                - mysqladmin
-                - ping
-                - -h
-                - localhost
+              command: ["sh","-c","mysqladmin ping -h localhost -uroot -p${MYSQL_ROOT_PASSWORD}"]

Review comment:
       @BLasan oh wait I am dumb, this `MYSQL_ROOT_PASSWORD` **IS** set by us, in https://github.com/apache/fineract/blob/develop/kubernetes/fineractmysql-deployment.yml#L92, now I get how this works! Then this change makes perfect sense - LGTM and I'm mergint it now. FYI @xurror 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [fineract] BLasan commented on pull request #1583: Fix MYSQL Access Denied Issue on restart

Posted by GitBox <gi...@apache.org>.
BLasan commented on pull request #1583:
URL: https://github.com/apache/fineract/pull/1583#issuecomment-764936401


   @vorburger Sir could you please take a look?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [fineract] BLasan commented on a change in pull request #1583: Fix MYSQL Restart Issue

Posted by GitBox <gi...@apache.org>.
BLasan commented on a change in pull request #1583:
URL: https://github.com/apache/fineract/pull/1583#discussion_r559565081



##########
File path: kubernetes/kubectl-startup.sh
##########
@@ -19,7 +19,7 @@
 #
 
 echo "Setting Up Fineract service configuration..."
-kubectl create secret generic fineract-tenants-db-secret --from-literal=username=root --from-literal=password=$(head /dev/urandom | tr -dc A-Za-z0-9 | head -c 16)
+kubectl create secret generic fineract-tenants-db-secret --from-literal=username=root --from-literal=password=root

Review comment:
       I have introduced an initcontainer instead of hardcoding the password. So before the container creation it will bind /var/lib/mysql directory which could help us to get rid of this "Access Denied" issue after restarting the container




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [fineract] BLasan commented on a change in pull request #1583: Fix MYSQL Restart Issue

Posted by GitBox <gi...@apache.org>.
BLasan commented on a change in pull request #1583:
URL: https://github.com/apache/fineract/pull/1583#discussion_r559565081



##########
File path: kubernetes/kubectl-startup.sh
##########
@@ -19,7 +19,7 @@
 #
 
 echo "Setting Up Fineract service configuration..."
-kubectl create secret generic fineract-tenants-db-secret --from-literal=username=root --from-literal=password=$(head /dev/urandom | tr -dc A-Za-z0-9 | head -c 16)
+kubectl create secret generic fineract-tenants-db-secret --from-literal=username=root --from-literal=password=root

Review comment:
       I have used an initcontainer instead of hardcoding the password. So before the container creation it will bind /var/lib/mysql directory which could help us to get rid of this "Access Denied" issue after restarting the container




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [fineract] BLasan commented on pull request #1583: Fix MYSQL Restart Issue

Posted by GitBox <gi...@apache.org>.
BLasan commented on pull request #1583:
URL: https://github.com/apache/fineract/pull/1583#issuecomment-762661961


   @vorburger  @xurror  This issue will occur if we do not remove volume mounted inside minikube node when cluster resetting. I've updated REDME.md and changed the liveness probe command which needs to run with root user credentials. Please review


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [fineract] BLasan commented on a change in pull request #1583: Fix MYSQL Restart Issue

Posted by GitBox <gi...@apache.org>.
BLasan commented on a change in pull request #1583:
URL: https://github.com/apache/fineract/pull/1583#discussion_r559565081



##########
File path: kubernetes/kubectl-startup.sh
##########
@@ -19,7 +19,7 @@
 #
 
 echo "Setting Up Fineract service configuration..."
-kubectl create secret generic fineract-tenants-db-secret --from-literal=username=root --from-literal=password=$(head /dev/urandom | tr -dc A-Za-z0-9 | head -c 16)
+kubectl create secret generic fineract-tenants-db-secret --from-literal=username=root --from-literal=password=root

Review comment:
       I have introduced an initcontainer instead of hardcoding the password. So before the container creation it will bind /var/lib/mysql directory which could help us to get rid of this "Access Denied" issue after restarting the container




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org