CodeSOD: Drop it Like it’s a Deployment


Zenith’s company went ahead on and outsourced 95% of their development to the lowest bidder. Said bidder promised a lot of XML and MVC and whatever TLAs sounded buzzwordy that day, and off they went. It’s okay, though, the custmoer isn’t just taking that code and deploying it- “Zenith” gets to do code reviews to ensure code quality. The general flow of the post-code-review conversation goes something like:

Zenith: This code shouldn’t go into production, hell, it’s so bad that a proud parent wouldn’t even hang it on their fridge.
Management: I’ll raise your concerns.
Outsourced Team: We did the needful, please review again.
Zenith: They didn’t change anything. It doesn’t even compile.
Offshore Team: There are too many barriers, we cannot hit deadlines, your team is too strict
Managment: Yeah… I guess you’re gonna have to lay off the contractors. Don’t be so strict in your code reviews. We have to deliver software!

The worst code ended up, not in the software, but in the deployment scripts. The team didn’t have and didn’t want a build environment (because they didn’t want to be expected to test their deployment scripts), so they essentially just guessed what the deployment scripts should be like and hoped for the best. They didn’t check them, they certainly didn’t run them.

For deploying changes to stored procedures, they got especially interested in using DROP commands, like so:

    DROP PROCEDURE [schema].[foo];
    CREATE PROCEDURE [schema].[foo] AS…

DROP statements destroy the object and any grants associated with it- meaning the permissions got wiped out with every deployment. After a long weekend cleaning up a botched deployment, “Zenith” gave them a template to follow. All they needed to do was plug their code into a script that would never drop, but instead create/alter as needed.

They… “adapted” his script to their own processes.

IF EXISTS(select * from sys.all_objects where name = 'USPCandidateSearchElectionInfo')
        DROP PROCEDURE CF.USPCandidateSearchElectionInfo
GO
IF OBJECT_ID('[CFO].[USPCandidateSearchElectionInfo]') IS NULL
BEGIN
EXECUTE('CREATE PROCEDURE [CFO].[USPCandidateSearchElectionInfo] AS BEGIN SELECT NULL; END');
END
GO
ALTER PROCEDURE [CF].[USPCandidateSearchElectionInfo]
 @Request XML
AS
BEGIN
--pages and pages of horrific code follow, the details of which are inconsequential
RETURN @@ROWCOUNT
END

Not only did they keep the DROP, thus defeating the entire reason why he had given them a script in the first place, they also couldn’t even get so far using the same name forr the procedure all the way through.

“Zenith” raised this with management, and was once again scolded: “Code reviews are supposed to facilitate development, not provide a barrier to deployments.”


[Advertisement] Release!
is a light card game about software and the people who make it. Play with 2-5 people, or up to 10 with two copies – only $9.95 shipped!

http://ift.tt/2wDyM79

Advertisements

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s